refactor: migrate no_empty_block#299
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the no_empty_block lint rule to utilize the updated custom lint API, migrating from CustomLintResolver to RuleContext and RuleVisitorRegistry. It also introduces a comprehensive unit test suite in no_empty_block_rule_test.dart to replace the previous manual test file. Feedback on this PR highlights a bug in the _isPrecedingCommentToDo helper method of NoEmptyBlockVisitor, which fails to traverse the linked list of preceding comments and may incorrectly report a lint if a TODO comment is not the first comment in the block.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
…o_empty_block rule
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the no_empty_block lint rule to align with the updated custom lint rule structure, registers it in the main plugin, and adds comprehensive unit tests. The review feedback suggests traversing up all ancestor declarations when checking for exclusions to correctly handle nested closures or local functions, and making the TODO comment check case-insensitive to improve usability.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
solid-danylosafonov
left a comment
There was a problem hiding this comment.
lgtm, one minor suggestion
| exclude: | ||
| - method_name: excludeMethod | ||
| - class_name: Exclude | ||
| method_name: excludeMethod |
There was a problem hiding this comment.
I think we should rename it so that we can be sure that it's not ignored because of the above - method_name: excludeMethod
| method_name: excludeMethod | |
| method_name: excludeClassMethod |
f32d2ae
into
solid-software:analysis_server_migration
Closes #254