feat: update const border radius rule #482

grafovdenis · 2021-09-30T16:15:05Z

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[ ] Bug fix
[ ] New rule
[x] Changes an existing rule
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

What changes did you make? (Give an overview)

Update doc
Update test example
Update test
Update rule visitor

codecov · 2021-09-30T16:47:44Z

Codecov Report

Merging #482 (6f97a03) into master (69678b6) will increase coverage by 0.04%.
The diff coverage is 86.47%.

❗ Current head 6f97a03 differs from pull request most recent head f472a80. Consider uploading reports for the commit f472a80 to get more accurate results

@@            Coverage Diff             @@
##           master     #482      +/-   ##
==========================================
+ Coverage   83.64%   83.68%   +0.04%     
==========================================
  Files         189      199      +10     
  Lines        3906     4124     +218     
==========================================
+ Hits         3267     3451     +184     
- Misses        639      673      +34

Impacted Files	Coverage Δ
...onst_border_radius/prefer_const_border_radius.dart	`100.00% <ø> (ø)`
lib/src/cli/commands/check_unused_l10n.dart	`57.14% <57.14%> (ø)`
lib/src/analyzers/lint_analyzer/lint_analyzer.dart	`91.74% <66.66%> (-1.84%)`	⬇️
lib/src/config_builder/config_builder.dart	`81.81% <77.77%> (-1.52%)`	⬇️
...yzers/unused_l10n_analyzer/unused_l10n_config.dart	`81.81% <81.81%> (ø)`
...ers/unused_l10n_analyzer/unused_l10n_analyzer.dart	`86.58% <86.58%> (ø)`
...zers/unused_l10n_analyzer/unused_l10n_visitor.dart	`90.90% <90.90%> (ø)`
...ics/metrics_list/maintainability_index_metric.dart	`100.00% <100.00%> (ø)`
...rics/metrics_list/number_of_parameters_metric.dart	`100.00% <100.00%> (ø)`
...list/source_lines_of_code/source_code_visitor.dart	`100.00% <100.00%> (ø)`
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 69678b6...f472a80. Read the comment docs.

lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_const_border_radius/visitor.dart

incendial

Thanks for the rule update. Added several small comments.

incendial · 2021-10-03T08:15:17Z

CHANGELOG.md


 ## Unreleased

+* feat: Update static code diagnostic `prefer-const-border-radius`.


Since we didn't release the rule yet, we don't need to mention the rule change.

incendial · 2021-10-03T08:25:03Z

...rs/lint_analyzer/rules/rules_list/prefer_const_border_radius/prefer_const_border_radius.dart

  static const ruleId = 'prefer-const-border-radius';
-  static const _issueMessage = 'Prefer use const constructor BorderRadius.all.';
+  static const _issueMessage =
+      'Prefer to use const constructor BorderRadius.all.';


Let's have either Prefer using const constructor BorderRadius.all or Prefer const constructor BorderRadius.all.. We don't use to use in any other rule.

incendial · 2021-10-03T08:25:19Z

lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_const_border_radius/visitor.dart


 const borderRadiusClassName = 'BorderRadius';
-const borderRadiusConstConstructorName = 'circular';
+const borderRadiusConstructorName = 'circular';


Please make these variables private.

incendial · 2021-10-03T08:26:02Z

test/analyzers/lint_analyzer/rules/rules_list/prefer_const_border_radius/examples/example.dart

@@ -1,47 +1,29 @@
-const _constRadius = BorderRadius.all(16);
-final _finalRadius = BorderRadius.circular(8); // LINT
+import 'flutter_define.dart';


Let's keep all the rule code in the example file.

incendial · 2021-10-03T08:27:13Z

test/analyzers/lint_analyzer/rules/rules_list/prefer_const_border_radius/examples/example.dart

+      ClipRRect(
+          borderRadius:
+              BorderRadius.circular(_constValue - _constValue)), // LINT
+      ClipRRect(borderRadius: BorderRadius.circular(_constValue - _finalValue)),


Could you add test cases for conditional expression as BorderRadius argument and for method invocation as argument?

incendial · 2021-10-03T08:29:05Z

...nt_analyzer/rules/rules_list/prefer_const_border_radius/prefer_const_border_radius_test.dart

-          startColumns: [22, 31, 31, 31],
-          endOffsets: [88, 195, 295, 442],
+          startOffsets: [113, 273, 373, 440, 588, 870],
+          startLines: [4, 9, 12, 13, 17, 22],


I see seven // LINT comments and only 6 actual triggers (line 25 doesn't trigger the rule). Please update the test file.

This reverts commit 77dcc80.

grafovdenis added 8 commits September 30, 2021 19:05

update doc

91402c2

update test example

634377b

update test

4220d8f

update _issueMessage

2b5aa3b

update visitor

600cf83

rm odd variable

c5dede1

format

147ead8

update test

77c5b8e

grafovdenis changed the title ~~Update const border radius rule~~ feat: update const border radius rule Sep 30, 2021

update changelog

77dcc80

dkrutskikh requested review from dkrutskikh and incendial September 30, 2021 16:39

dkrutskikh suggested changes Sep 30, 2021

View reviewed changes

lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_const_border_radius/visitor.dart Outdated Show resolved Hide resolved

rm todo

999a0b9

dkrutskikh approved these changes Oct 1, 2021

View reviewed changes

incendial suggested changes Oct 3, 2021

View reviewed changes

incendial added the type: enhancement New feature or request label Oct 3, 2021

incendial added this to the 4.4.0 milestone Oct 3, 2021

grafovdenis added 4 commits October 4, 2021 10:27

Revert "update changelog"

7cf22e5

This reverts commit 77dcc80.

update issue message

892bfcc

make fields private

f700334

update test

9568df6

grafovdenis requested a review from incendial October 4, 2021 07:38

update test & example

f472a80

incendial approved these changes Oct 4, 2021

View reviewed changes

dkrutskikh merged commit 624874f into dart-code-checker:master Oct 4, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: update const border radius rule #482

feat: update const border radius rule #482

Uh oh!

grafovdenis commented Sep 30, 2021 •

edited

Loading

Uh oh!

codecov bot commented Sep 30, 2021 •

edited

Loading

Uh oh!

Uh oh!

incendial left a comment

Uh oh!

incendial Oct 3, 2021

Uh oh!

incendial Oct 3, 2021

Uh oh!

incendial Oct 3, 2021

Uh oh!

incendial Oct 3, 2021

Uh oh!

incendial Oct 3, 2021

Uh oh!

incendial Oct 3, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants


		## Unreleased

		* feat: Update static code diagnostic `prefer-const-border-radius`.

feat: update const border radius rule #482

feat: update const border radius rule #482

Uh oh!

Conversation

grafovdenis commented Sep 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is the purpose of this pull request? (put an "X" next to an item)

What changes did you make? (Give an overview)

Uh oh!

codecov bot commented Sep 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

incendial left a comment

Choose a reason for hiding this comment

Uh oh!

incendial Oct 3, 2021

Choose a reason for hiding this comment

Uh oh!

incendial Oct 3, 2021

Choose a reason for hiding this comment

Uh oh!

incendial Oct 3, 2021

Choose a reason for hiding this comment

Uh oh!

incendial Oct 3, 2021

Choose a reason for hiding this comment

Uh oh!

incendial Oct 3, 2021

Choose a reason for hiding this comment

Uh oh!

incendial Oct 3, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

grafovdenis commented Sep 30, 2021 •

edited

Loading

codecov bot commented Sep 30, 2021 •

edited

Loading