From 6cbeb5d4ad1fdc2c5d8c15cdba86aa7a60d9e377 Mon Sep 17 00:00:00 2001 From: Istvan Soos Date: Fri, 10 Jan 2025 10:38:14 +0100 Subject: [PATCH] Use explicit URL issue acceptance status and display ones with 404 status in the results. --- CHANGELOG.md | 2 + lib/src/package_analyzer.dart | 10 ++-- lib/src/references/pubspec_urls.dart | 54 +++++++++++--------- lib/src/report/template.dart | 21 +++----- test/goldens/end2end/url_launcher-6.3.1.json | 1 + 5 files changed, 45 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d152723b..198dcb68 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ - Make example's `README.md` priority over any dart file in the examples directory. - Expose more precise repository verification status in `AnalysisResult`. +- Track URL issues with explicit acceptance state (404 status does not cause score deduction). + Also display such URLs in `AnalysisResult`. ## 0.22.17 diff --git a/lib/src/package_analyzer.dart b/lib/src/package_analyzer.dart index 09d59bb4..b3e3f03d 100644 --- a/lib/src/package_analyzer.dart +++ b/lib/src/package_analyzer.dart @@ -260,12 +260,12 @@ Future _createAnalysisResult( final repoVerification = await context.repository; final repository = repoVerification.repository; final fundingUrls = - pubspecUrls.funding.map((e) => e.verifiedUrl).nonNulls.toList(); + pubspecUrls.funding.map((e) => e.acceptedUrl).nonNulls.toList(); return AnalysisResult( - homepageUrl: pubspecUrls.homepage.verifiedUrl, - repositoryUrl: pubspecUrls.repository.verifiedUrl, - issueTrackerUrl: pubspecUrls.issueTracker.verifiedUrl, - documentationUrl: pubspecUrls.documentation.verifiedUrl, + homepageUrl: pubspecUrls.homepage.acceptedUrl, + repositoryUrl: pubspecUrls.repository.acceptedUrl, + issueTrackerUrl: pubspecUrls.issueTracker.acceptedUrl, + documentationUrl: pubspecUrls.documentation.acceptedUrl, fundingUrls: fundingUrls.isEmpty ? null : fundingUrls, repositoryStatus: repoVerification.status, repository: repository, diff --git a/lib/src/references/pubspec_urls.dart b/lib/src/references/pubspec_urls.dart index c067339a..e3f37d6b 100644 --- a/lib/src/references/pubspec_urls.dart +++ b/lib/src/references/pubspec_urls.dart @@ -23,24 +23,31 @@ class PubspecUrlsWithIssues { required this.funding, }); - Iterable get issues => [ - homepage.issue, - repository.issue, - issueTracker.issue, - documentation.issue, - ...funding.map((i) => i.issue), - ].whereType(); + late final _withIssues = [ + homepage, + repository, + issueTracker, + documentation, + ...funding, + ].where((e) => e.issue != null).toList(); + + Iterable get issues => _withIssues.map((e) => e.issue).nonNulls; + + late final hasRejected = _withIssues.any((e) => !e.isAccepted); } class UrlWithIssue { final String? url; final Issue? issue; - UrlWithIssue(this.url, this.issue); + /// While the [url] may have issues, we could still display it, as the problem may be transient. + final bool isAccepted; + + UrlWithIssue.accepted(this.url, {this.issue}) : isAccepted = true; + UrlWithIssue.rejected(this.issue, {this.url}) : isAccepted = false; - late final isOK = url != null && issue == null; - late final isNotOK = !isOK; - late final verifiedUrl = isOK ? url : null; + late final isVerified = url != null && isAccepted && issue == null; + late final acceptedUrl = isAccepted ? url : null; } /// Verifies the URLs in pubspec and builds the [PubspecUrlsWithIssues] object. @@ -81,7 +88,7 @@ Future checkPubspecUrls(PackageContext context) async { ).toString(); final inferredResult = await _checkUrl( context, 'issue_tracker', 'Issue tracker URL', inferredUrl); - if (inferredResult.isOK) { + if (inferredResult.isVerified) { issueTracker = inferredResult; } } @@ -111,8 +118,7 @@ Future _checkUrlInPubspec( final pubspec = context.pubspec; final content = pubspec.originalYaml[key]; if (content != null && content is! String) { - return UrlWithIssue( - null, + return UrlWithIssue.rejected( Issue('The `$key` entry, if present, should be a string containing a url', span: tryGetSpanFromYamlMap(pubspec.originalYaml, key)), ); @@ -121,10 +127,10 @@ Future _checkUrlInPubspec( if (url == null || url.isEmpty) { if (isRequired) { - return UrlWithIssue( - null, Issue("`pubspec.yaml` doesn't have a `$key` entry.")); + return UrlWithIssue.rejected( + Issue("`pubspec.yaml` doesn't have a `$key` entry.")); } - return UrlWithIssue(null, null); + return UrlWithIssue.rejected(null); } return await _checkUrl(context, key, name, url); } @@ -138,17 +144,17 @@ Future _checkUrl( final pubspec = context.pubspec; final status = await context.sharedContext.checkUrlStatus(url); if (status.isInvalid) { - return UrlWithIssue( - url, + return UrlWithIssue.rejected( Issue( "$name isn't valid.", span: tryGetSpanFromYamlMap(pubspec.originalYaml, key), ), + url: url, ); } else if (!status.exists) { - return UrlWithIssue( + return UrlWithIssue.accepted( url, - Issue( + issue: Issue( "$name doesn't exist.", span: tryGetSpanFromYamlMap(pubspec.originalYaml, key), suggestion: 'At the time of the analysis `$url` was unreachable. ' @@ -156,8 +162,8 @@ Future _checkUrl( ), ); } else if (!status.isSecure) { - return UrlWithIssue( - url, + return UrlWithIssue.rejected( + url: url, Issue( '$name is insecure.', span: tryGetSpanFromYamlMap(pubspec.originalYaml, key), @@ -169,5 +175,5 @@ Future _checkUrl( if (problemCode != null) { context.urlProblems[url] = problemCode; } - return UrlWithIssue(url, null); + return UrlWithIssue.accepted(url); } diff --git a/lib/src/report/template.dart b/lib/src/report/template.dart index a599e58e..68827721 100644 --- a/lib/src/report/template.dart +++ b/lib/src/report/template.dart @@ -166,9 +166,6 @@ Future followsTemplate(PackageContext context) async { )); } - final pubspecUrls = await context.pubspecUrlsWithIssues; - issues.addAll(pubspecUrls.issues); - final repository = await context.repository; final repositoryStatus = repository.status; if (repositoryStatus == RepositoryStatus.failed) { @@ -197,20 +194,16 @@ Future followsTemplate(PackageContext context) async { } } - final unreachableFailuresIgnored = - // every issue is about an URL being unreachable - issues.isNotEmpty && - issues.every((issue) => - (issue.suggestion ?? '').contains('was unreachable')) && - // repository verification succeeded - repository.repository != null; + final pubspecUrls = await context.pubspecUrlsWithIssues; + // Checks the issues identified so far. + final hasAnyNonUrlIssue = issues.isNotEmpty; + issues.addAll(pubspecUrls.issues); + final hasFailure = hasAnyNonUrlIssue || pubspecUrls.hasRejected; final status = issues.isEmpty ? ReportStatus.passed - : (unreachableFailuresIgnored - ? ReportStatus.partial - : ReportStatus.failed); - final points = (issues.isEmpty || unreachableFailuresIgnored) ? 10 : 0; + : (hasFailure ? ReportStatus.failed : ReportStatus.partial); + final points = hasFailure ? 0 : 10; return Subsection( 'Provide a valid `pubspec.yaml`', issues, diff --git a/test/goldens/end2end/url_launcher-6.3.1.json b/test/goldens/end2end/url_launcher-6.3.1.json index 9d148110..a5d14fd7 100644 --- a/test/goldens/end2end/url_launcher-6.3.1.json +++ b/test/goldens/end2end/url_launcher-6.3.1.json @@ -167,6 +167,7 @@ "screenshots": [], "result": { "repositoryUrl": "https://github.com/flutter/packages/tree/main/packages/url_launcher/url_launcher", + "issueTrackerUrl": "https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+url_launcher%22", "repositoryStatus": "verified", "repository": { "provider": "github",