Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use explicit URL issue status and display the accepted ones in the results. #1434

Merged
merged 1 commit into from
Jan 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
10 changes: 5 additions & 5 deletions lib/src/package_analyzer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -260,12 +260,12 @@ Future<AnalysisResult> _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,
Expand Down
54 changes: 30 additions & 24 deletions lib/src/references/pubspec_urls.dart
Original file line number Diff line number Diff line change
Expand Up @@ -23,24 +23,31 @@ class PubspecUrlsWithIssues {
required this.funding,
});

Iterable<Issue> get issues => [
homepage.issue,
repository.issue,
issueTracker.issue,
documentation.issue,
...funding.map((i) => i.issue),
].whereType<Issue>();
late final _withIssues = [
homepage,
repository,
issueTracker,
documentation,
...funding,
].where((e) => e.issue != null).toList();

Iterable<Issue> 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.
Expand Down Expand Up @@ -81,7 +88,7 @@ Future<PubspecUrlsWithIssues> checkPubspecUrls(PackageContext context) async {
).toString();
final inferredResult = await _checkUrl(
context, 'issue_tracker', 'Issue tracker URL', inferredUrl);
if (inferredResult.isOK) {
if (inferredResult.isVerified) {
issueTracker = inferredResult;
}
}
Expand Down Expand Up @@ -111,8 +118,7 @@ Future<UrlWithIssue> _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)),
);
Expand All @@ -121,10 +127,10 @@ Future<UrlWithIssue> _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);
}
Expand All @@ -138,26 +144,26 @@ Future<UrlWithIssue> _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. '
'Make sure that the website is reachable via [`HEAD`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods/HEAD) requests.',
),
);
} else if (!status.isSecure) {
return UrlWithIssue(
url,
return UrlWithIssue.rejected(
url: url,
Issue(
'$name is insecure.',
span: tryGetSpanFromYamlMap(pubspec.originalYaml, key),
Expand All @@ -169,5 +175,5 @@ Future<UrlWithIssue> _checkUrl(
if (problemCode != null) {
context.urlProblems[url] = problemCode;
}
return UrlWithIssue(url, null);
return UrlWithIssue.accepted(url);
}
21 changes: 7 additions & 14 deletions lib/src/report/template.dart
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,6 @@ Future<ReportSection> 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) {
Expand Down Expand Up @@ -197,20 +194,16 @@ Future<ReportSection> 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,
Expand Down
1 change: 1 addition & 0 deletions test/goldens/end2end/url_launcher-6.3.1.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Loading