docs: improve avoid_duplicate_code rule and parameter documentation - #337
Conversation
…ith detailed explanations and examples
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughExpanded documentation for duplicate-code detection and its configuration parameters, including examples and updated sample settings. No rule logic, public API signatures, parsing, serialization, or equality behavior changed. ChangesDuplicate Code Documentation
Estimated code review effort: 1 (Trivial) | ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule.dart`:
- Around line 8-13: Qualify the documentation in the avoid_duplicate_code rule
comment to state that it reports all currently known copies, rather than
implying every cross-file copy is always reported. Update the wording near the
rule description and the related explanation around the first-file/deferred
analysis behavior, without changing detection logic.
- Around line 18-23: Update the Type 2 clones documentation in the avoid
duplicate code rule to state that literal values are normalized only when
ignore_literals is enabled, while local variables and formal parameters are
normalized only when ignore_identifiers is enabled; avoid implying these
differences are always ignored.
- Around line 57-61: Update the Persistent Disk Cache documentation to describe
the stored data accurately as candidate hash entries and metadata, including
modification stamps and HashEntry details such as hashes, offsets, lengths, and
token counts; remove the claim that AST block structures are persisted.
- Around line 75-80: Update the inline analysis-ignore example in the
documentation for avoid_duplicate_code so the myBoilerplateMethod declaration
contains syntactically valid Dart, using a real method body rather than the
invalid `{ ... }` placeholder; keep the surrounding ignore-comment guidance
unchanged.
- Around line 35-39: Update the Structural Hashing documentation comment near
the avoid-duplicate-code rule to remove the PMD CPD and SonarQube attribution,
leaving only accurate description of the Bob Jenkins one-at-a-time hash
algorithm and its use for AST subtree fingerprints.
In
`@lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart`:
- Around line 104-131: The checkBlocks documentation example must align with the
default minTokens threshold and actual traversal behavior. Update the example so
the duplicated block body contains at least 30 tokens, and revise the
check_blocks false description to state that nested `{ ... }` Block bodies are
skipped while expression-bodied functions remain checked; preserve the
documented duplicate outcome.
- Around line 20-28: Update the documentation comparison table in
AvoidDuplicateCodeParameters to use accurate DCM and SonarQube defaults,
distinguishing SonarQube’s Java statement threshold from its non-Java token/line
thresholds; otherwise remove those rows if equivalent defaults cannot be
represented consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 403c864a-72fc-41c6-a6b0-1168f5e4cf3f
📒 Files selected for processing (2)
lib/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule.dartlib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart
…age context and clarified examples
| /// * **Coincidental Duplication:** Not all duplication is harmful. If two | ||
| /// identical code blocks serve completely distinct business purposes, merging | ||
| /// them creates a forced dependency. This can lead to | ||
| /// [premature abstraction](https://medium.com/@ricrivero3/premature-abstraction-is-the-root-of-all-evil-7309762c0635) |
There was a problem hiding this comment.
I don't like this reference, it's not as informative and feels a bit biased? We should link to research or wikipedia.
There was a problem hiding this comment.
Updated:
/// * **Coincidental Duplication:** Not all duplication is harmful. If two
/// identical code blocks serve completely distinct business purposes, merging
/// them creates a forced dependency. This can lead to
/// [hasty abstractions][aha] and [tight coupling][coupling].
///
/// :::
///
/// [aha]: https://en.wikipedia.org/wiki/Don't_repeat_yourself#AHA
/// [coupling]: https://en.wikipedia.org/wiki/Coupling_(computer_programming)
Summary by CodeRabbit
avoid_duplicate_codeexample configuration: updatedminTokens, enabledcheckBlocks, and revisedexcludeentries.