Skip to content

[analysis] Migrate issue and repository link syntax rules to flutter_analyzer_plugin - #191127

Merged
auto-submit[bot] merged 7 commits into
flutter:masterfrom
bkonyi:migrate_link_syntax_rules
Aug 15, 2026
Merged

auto-submit[bot] merged 7 commits into
flutter:masterfrom
bkonyi:migrate_link_syntax_rules

Conversation

@bkonyi

@bkonyi bkonyi commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Migrates verifyIssueLinks and verifyRepositoryLinks from dev/bots/analyze.dart to AST-based AnalysisRules in dev/flutter_analyzer_plugin.

Changes

  • Implements IssueLinkSyntax in dev/flutter_analyzer_plugin/lib/src/rules/issue_link_syntax.dart.
  • Implements RepositoryLinkSyntax in dev/flutter_analyzer_plugin/lib/src/rules/repository_link_syntax.dart.
  • Adds unit tests in dev/flutter_analyzer_plugin/test/issue_link_syntax_test.dart and dev/flutter_analyzer_plugin/test/repository_link_syntax_test.dart.
  • Registers both rules in dev/flutter_analyzer_plugin/lib/main.dart.
  • Removes verifyIssueLinks, verifyRepositoryLinks, 'issue-links', and 'repository-links' from dev/bots/analyze.dart and dev/bots/test/analyze_test.dart.

@flutter-dashboard flutter-dashboard Bot added the CICD Run CI/CD label Aug 14, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request migrates the issue link and repository link validation checks from the legacy analyze.dart script into the flutter_analyzer_plugin as two new analyzer rules: IssueLinkSyntax and RepositoryLinkSyntax. The review feedback suggests preventing IssueLinkSyntax from self-triggering on its own source file, and adding support for analyzing adjacent string literals in both rules to ensure split links are correctly validated without double-reporting.

Comment thread dev/flutter_analyzer_plugin/lib/src/rules/issue_link_syntax.dart Outdated
Comment thread dev/flutter_analyzer_plugin/lib/src/rules/issue_link_syntax.dart
Comment thread dev/flutter_analyzer_plugin/lib/src/rules/issue_link_syntax.dart
Comment thread dev/flutter_analyzer_plugin/lib/src/rules/issue_link_syntax.dart
@github-actions github-actions Bot added framework flutter/packages/flutter repository. See also f: labels. f: scrolling Viewports, list views, slivers, etc. labels Aug 14, 2026
@bkonyi
bkonyi requested a review from srawlins August 15, 2026 00:01
final AnalysisRule rule;
final RuleContext context;

bool _shouldSkip() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW we sometimes put checks like this all the way up in the registerNodeProcessors code, so that the rule isn't even registered for files that aren't meant to be considered. Saves performance.

@bkonyi bkonyi added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 15, 2026
@auto-submit
auto-submit Bot added this pull request to the merge queue Aug 15, 2026
Merged via the queue into flutter:master with commit b0319d8 Aug 15, 2026
22 checks passed
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Aug 15, 2026
pull Bot pushed a commit to NOUIY/flutter that referenced this pull request Aug 16, 2026
…le path (flutter#191159)

## Description

This PR optimizes static analysis performance in
`dev/flutter_analyzer_plugin` by early-exiting in
`registerNodeProcessors` based on file path for rules that only target
specific files:
- `IssueLinkSyntax`: Early exits for unit tests (`_test.dart`) and the
rule's definition file (`issue_link_syntax.dart`), removing
`_shouldSkip()` from the visitor.
- `IntegrationTestTimeouts`: Early exits for non-integration-test files
(files not containing `test_driver` or `test.dart`), removing the
redundant per-invocation file path check in `visitMethodInvocation`.
- `NoBadImportsInFlutter`: Early exits for files not in
`packages/flutter/lib/src/`, removing redundant path checks in
`visitImportDirective`.

This avoids unnecessary AST visitor registration and listener callbacks
across non-applicable files in the repository.

Addresses feedback from @srawlins in flutter#191127.

## Related Issues
- Follow-up to flutter#191127

## Tests
- All existing tests in `dev/flutter_analyzer_plugin/test/` continue to
pass.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CICD Run CI/CD f: scrolling Viewports, list views, slivers, etc. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants