diff --git a/app/lib/dartdoc/dartdoc_runner.dart b/app/lib/dartdoc/dartdoc_runner.dart index 9433a8d538..d299c8f5bf 100644 --- a/app/lib/dartdoc/dartdoc_runner.dart +++ b/app/lib/dartdoc/dartdoc_runner.dart @@ -3,6 +3,7 @@ // BSD-style license that can be found in the LICENSE file. import 'dart:async'; +import 'dart:convert' as convert; import 'dart:io'; import 'package:logging/logging.dart'; @@ -26,6 +27,7 @@ import '../shared/versions.dart' as versions; import 'backend.dart'; import 'customization.dart'; import 'models.dart'; +import 'pub_dartdoc_data.dart'; final Logger _logger = new Logger('pub.dartdoc.runner'); final Uuid _uuid = new Uuid(); @@ -34,9 +36,14 @@ const statusFilePath = 'status.json'; const _archiveFilePath = 'package.tar.gz'; const _buildLogFilePath = 'log.txt'; const _packageTimeout = const Duration(minutes: 10); +const _pubDataFileName = 'pub-data.json'; const _sdkTimeout = const Duration(minutes: 20); final Duration _twoYears = const Duration(days: 2 * 365); +// We'll emit a suggestion and apply score penalty only if the coverage is below +// this value. +const _coverageEmitThreshold = 0.1; + final _pkgPubDartdocDir = Platform.script.resolve('../../pkg/pub_dartdoc').toFilePath(); @@ -73,7 +80,7 @@ class DartdocJobProcessor extends JobProcessor { timeout: _sdkTimeout, ); - final pubDataFile = new File(p.join(outputDir, 'pub-data.json')); + final pubDataFile = new File(p.join(outputDir, _pubDataFileName)); final hasPubData = await pubDataFile.exists(); final isOk = pr.exitCode == 0 && hasPubData; if (!isOk) { @@ -124,7 +131,8 @@ class DartdocJobProcessor extends JobProcessor { bool hasContent = false; String reportStatus = ReportStatus.failed; - final suggestions = []; + final healthSuggestions = []; + final maintenanceSuggestions = []; try { final pkgDir = await downloadPackage(job.packageName, job.packageVersion); if (pkgDir == null) { @@ -197,17 +205,52 @@ class DartdocJobProcessor extends JobProcessor { await toolEnvRef.release(); } - if (!hasContent) { - suggestions.add(getDartdocRunFailedSuggestion()); + double coverageScore = 0.0; + final dartdocData = await _loadPubDartdocData(outputDir); + if (hasContent && dartdocData != null) { + final total = dartdocData.apiElements.length; + final documented = dartdocData.apiElements + .where((elem) => + elem.documentation != null && + elem.documentation.isNotEmpty && + elem.documentation.trim().length >= 5) + .length; + if (total == documented) { + // this also handles total == 0 + coverageScore = 1.0; + } else { + coverageScore = documented / total; + } + + if (coverageScore < _coverageEmitThreshold) { + final level = coverageScore < 0.2 + ? SuggestionLevel.warning + : SuggestionLevel.hint; + final undocumented = total - documented; + healthSuggestions.add( + new Suggestion( + 'dartdoc.coverage', // TODO: extract as const in pana + level, + 'Document public APIs', + '$undocumented out of $total API elements (library, class, field ' + 'or method) have no adequate dartdoc content. Good documentation ' + 'improves code readability and discoverability through search.', + score: (1.0 - coverageScore) * 10.0), + ); + } + } else { + maintenanceSuggestions.add(getDartdocRunFailedSuggestion()); } - // TODO: calculate coverage score await scoreCardBackend.updateReport( job.packageName, job.packageVersion, new DartdocReport( reportStatus: reportStatus, - coverageScore: hasContent ? 1.0 : 0.0, - suggestions: suggestions.isEmpty ? null : suggestions, + coverageScore: coverageScore, + healthSuggestions: + healthSuggestions.isEmpty ? null : healthSuggestions, + maintenanceSuggestions: + maintenanceSuggestions.isEmpty ? null : maintenanceSuggestions, )); await scoreCardBackend.updateScoreCard(job.packageName, job.packageVersion); @@ -381,4 +424,19 @@ class DartdocJobProcessor extends JobProcessor { await tmpTar.rename(p.join(outputDir, _archiveFilePath)); _appendLog(logFileOutput, pr); } + + Future _loadPubDartdocData(String outputDir) async { + final file = new File(p.join(outputDir, _pubDataFileName)); + if (!file.existsSync()) { + return null; + } + try { + final content = await file.readAsString(); + return new PubDartdocData.fromJson( + convert.json.decode(content) as Map); + } catch (e, st) { + _logger.warning('Unable to parse $_pubDataFileName.', e, st); + return null; + } + } } diff --git a/app/lib/scorecard/models.dart b/app/lib/scorecard/models.dart index bddbade7ad..ef9ac10d5e 100644 --- a/app/lib/scorecard/models.dart +++ b/app/lib/scorecard/models.dart @@ -4,6 +4,7 @@ import 'dart:convert'; import 'dart:io'; +import 'dart:math' as math; import 'package:gcloud/db.dart' as db; import 'package:json_annotation/json_annotation.dart'; @@ -150,9 +151,14 @@ class ScoreCard extends db.ExpandoModel { PanaReport panaReport, DartdocReport dartdocReport, }) { - healthScore = (panaReport?.healthScore ?? 0.0) * - (0.9 + ((dartdocReport?.coverageScore ?? 1.0) * 0.1)); - maintenanceScore = panaReport?.maintenanceScore ?? 0.0; + healthScore = _applySuggestions( + panaReport?.healthScore ?? 0.0, + dartdocReport?.healthSuggestions, + ); + maintenanceScore = _applySuggestions( + panaReport?.maintenanceScore ?? 0.0, + dartdocReport?.maintenanceSuggestions, + ); platformTags = panaReport?.platformTags ?? []; reportTypes = [ panaReport == null ? null : ReportType.pana, @@ -162,6 +168,15 @@ class ScoreCard extends db.ExpandoModel { ..sort(); panaReport?.flags?.forEach(addFlag); } + + double _applySuggestions(double score, List suggestions) { + suggestions?.forEach((s) { + if (s.score != null) { + score -= s.score / 100.0; + } + }); + return math.max(score, 0.0); + } } /// Detail of a specific report for a given PackageVersion. @@ -381,12 +396,19 @@ class DartdocReport implements ReportData { final double coverageScore; - final List suggestions; + /// Suggestions related to the package health score. + @JsonKey(includeIfNull: false) + final List healthSuggestions; + + /// Suggestions related to the package maintenance score. + @JsonKey(includeIfNull: false) + final List maintenanceSuggestions; DartdocReport({ @required this.reportStatus, @required this.coverageScore, - @required this.suggestions, + @required this.healthSuggestions, + @required this.maintenanceSuggestions, }); factory DartdocReport.fromJson(Map json) => diff --git a/app/lib/scorecard/models.g.dart b/app/lib/scorecard/models.g.dart index 46453bf708..8f9388b6ec 100644 --- a/app/lib/scorecard/models.g.dart +++ b/app/lib/scorecard/models.g.dart @@ -117,15 +117,29 @@ DartdocReport _$DartdocReportFromJson(Map json) { return DartdocReport( reportStatus: json['reportStatus'] as String, coverageScore: (json['coverageScore'] as num)?.toDouble(), - suggestions: (json['suggestions'] as List) + healthSuggestions: (json['healthSuggestions'] as List) + ?.map((e) => + e == null ? null : Suggestion.fromJson(e as Map)) + ?.toList(), + maintenanceSuggestions: (json['maintenanceSuggestions'] as List) ?.map((e) => e == null ? null : Suggestion.fromJson(e as Map)) ?.toList()); } -Map _$DartdocReportToJson(DartdocReport instance) => - { - 'reportStatus': instance.reportStatus, - 'coverageScore': instance.coverageScore, - 'suggestions': instance.suggestions - }; +Map _$DartdocReportToJson(DartdocReport instance) { + var val = { + 'reportStatus': instance.reportStatus, + 'coverageScore': instance.coverageScore, + }; + + void writeNotNull(String key, dynamic value) { + if (value != null) { + val[key] = value; + } + } + + writeNotNull('healthSuggestions', instance.healthSuggestions); + writeNotNull('maintenanceSuggestions', instance.maintenanceSuggestions); + return val; +}