feat: add static code diagnostic consistent-update-render-object #1004

incendial · 2022-09-18T09:07:08Z

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

…-object-setters

incendial · 2022-09-18T09:07:33Z

@fzyzcjy again kindly asking you to take a look 🙂

incendial · 2022-09-18T09:22:23Z

fzyzcjy · 2022-09-18T11:30:49Z

Let me put a tick box to track it:

Looks like a bug, but it is not revealed b/c the users of this render object never update widget with new value? Anyway bug is bug and should be fixed

https://github.com/flutter/flutter/blob/d72daf602de72a096b9f5c76468e95b69033412c/packages/flutter/lib/src/cupertino/slider.dart#L303 - constants too

Maybe we should follow this code, and add an assertion.

https://github.com/flutter/flutter/blob/b0aa50255b117dd6afd178ddae7f0c7b4a01dc11/packages/flutter/lib/src/cupertino/switch.dart#L377 - same thing with state

Maybe we should follow this code, and add an assertion.

https://github.com/flutter/flutter/blob/c150c5a03dd8f17d050521a2bd94e61d6b56f997/packages/flutter/lib/src/material/time_picker.dart#L698 - bug?

Dnk why orientation is a final field in render object. IMHO it should be modifyable. So I suspect it is bug.

https://github.com/flutter/flutter/blob/082ad11422dae822c47808337e18a9d2af5734eb/packages/flutter/lib/src/material/tabs.dart#L302 - oh, that would be hard to handle
https://github.com/flutter/flutter/blob/d3bc2bbcdd814beb343719d9b7fed58b3807675f/packages/flutter/lib/src/material/range_slider.dart#L712 - also state thing

Also maybe add assertion

https://github.com/flutter/flutter/blob/2aa348b9407e96ffe4eca8e8f213c7984afad3f7/packages/flutter/lib/src/widgets/sliver_persistent_header.dart#L393-L394 - no update, bug?

Agree bug.

https://github.com/flutter/flutter/blob/2aa348b9407e96ffe4eca8e8f213c7984afad3f7/packages/flutter/lib/src/widgets/sliver_persistent_header.dart#L413-L414 - same

Agree bug.

https://github.com/flutter/flutter/blob/15739345da5a077bb54a403a505994bffae79783/packages/flutter/lib/src/widgets/basic.dart#L3422-L3423 - also no update, but looks like a false positive, since no fields
https://github.com/flutter/flutter/blob/15739345da5a077bb54a403a505994bffae79783/packages/flutter/lib/src/widgets/basic.dart#L3506-L3507 - same
https://github.com/flutter/flutter/blob/c58dca2a45d2c33c1f206db80519306371ee0eed/packages/flutter/lib/src/widgets/editable_text.dart#L3749 - missing backgroundCursorColor, bug?

Agree - and this is very commonly used (the text field!)

https://github.com/flutter/flutter/blob/2aa348b9407e96ffe4eca8e8f213c7984afad3f7/packages/flutter/lib/src/widgets/sliver_prototype_extent_list.dart#L52 - missing update, bug?

childManager in parent is a final field, and I suspect buildcontext will not change (indeed it is the element). so I guess no bug?

https://github.com/flutter/flutter/blob/51bcdb9407211ccaf0616533fe1f9eb0e8f04fca/packages/flutter/lib/src/widgets/performance_overlay.dart#L115 - missing few fields, bug?

Agree bug. Btw this bug is 5yr old!

fzyzcjy · 2022-09-18T11:48:54Z

Aha this rule catches many more bugs! And they cause more problems - because the bug we caught w/ our previous lint rule only affect performance, while this rule catches logically wrong bugs. Great job!

So, same as before, shall I make a PR to flutter?

incendial · 2022-09-18T11:51:43Z

I'll check your response a bit later (I didn't list all the duplicate cases) and maybe add more links. After that, sure

fzyzcjy · 2022-09-18T11:52:12Z

Sure, take your time!

github-actions · 2022-09-19T18:07:16Z

Dart Code Metrics unused files report of dart_code_metrics. ✅

Summary

Scanned package folders: bin, example, lib
No unused files found! ✅

github-actions · 2022-09-19T18:07:19Z

Dart Code Metrics analyze report of dart_code_metrics. ✅

Summary

Scanned folders: bin, example, lib, test
Total scanned files: 509
Total lines of source code: 8805
Total classes: 385
Average Cyclomatic Number per line of code: 0.36 / 1
Average Source Lines of Code per method: 6
Total tech debt: 2170.0 hours
Found issues: 6 ⚠

codecov · 2022-09-19T18:28:31Z

Codecov Report

Merging #1004 (c0f3ae2) into master (e773d30) will increase coverage by 0.13%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1004      +/-   ##
==========================================
+ Coverage   87.62%   87.75%   +0.13%     
==========================================
  Files         320      322       +2     
  Lines        6739     6812      +73     
==========================================
+ Hits         5905     5978      +73     
  Misses        834      834

Impacted Files	Coverage Δ
...c/analyzers/lint_analyzer/rules/rules_factory.dart	`71.42% <ø> (ø)`
...r_object/consistent_update_render_object_rule.dart	`100.00% <100.00%> (ø)`
..._list/consistent_update_render_object/visitor.dart	`100.00% <100.00%> (ø)`
lib/src/utils/flutter_types_utils.dart	`100.00% <100.00%> (ø)`

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

fzyzcjy · 2022-09-19T23:18:34Z

I'll check your response a bit later (I didn't list all the duplicate cases) and maybe add more links. After that, sure

So I guess we are ready to file issues? :) (Given all those git updates)

incendial · 2022-09-20T06:08:08Z

I'll check your response a bit later (I didn't list all the duplicate cases) and maybe add more links. After that, sure

So I guess we are ready to file issues? :) (Given all those git updates)

Sorry, no, I did everything easy and left this one for today 😄 (I also want to clean the rule from false-positives)

fzyzcjy · 2022-09-20T06:12:43Z

Sure, take your time!

incendial · 2022-09-20T16:53:05Z

@fzyzcjy supported most of the false positives, checking the source code again

incendial · 2022-09-20T17:01:58Z

Edge case https://github.com/flutter/flutter/blob/f614c597022fd1fafca698f2fd824a707223c84e/packages/flutter/lib/src/widgets/framework.dart#L4760 ?

incendial · 2022-09-20T17:03:06Z

Left several edge cases not supported (assertions, a super call, method calls, constants) and looks like it's okay to have them.

sonarcloud · 2022-09-20T17:13:38Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

fzyzcjy · 2022-09-20T23:05:01Z

Edge case https://github.com/flutter/flutter/blob/f614c597022fd1fafca698f2fd824a707223c84e/packages/flutter/lib/src/widgets/framework.dart#L4760 ?

Think so. That field is final, so no need to update

incendial added 4 commits September 16, 2022 15:58

feat: add static code diagnostic prefer-checking-for-equals-in-render…

f27c157

…-object-setters

chore: rename the rule

bdd9f45

fix: handle not equal check

f5861ae

feat: add static code diagnostic consistent-update-render-object

180e19e

incendial added type: enhancement New feature or request area-rules labels Sep 18, 2022

incendial added this to the 4.19.0 milestone Sep 18, 2022

incendial requested a review from dkrutskikh September 18, 2022 09:07

incendial self-assigned this Sep 18, 2022

Base automatically changed from prefer-checking-for-equls-in-render-object-setters to master September 19, 2022 18:03

Merge branch 'master' into consistent-update-render-object

b698b66

incendial added 2 commits September 20, 2022 21:04

fix: exlude several edge cases

bfe2edc

chore: fix typo

c0f3ae2

incendial merged commit b235f78 into master Sep 20, 2022

incendial deleted the consistent-update-render-object branch September 20, 2022 17:49

fzyzcjy mentioned this pull request Sep 21, 2022

RenderObject is not updated when users provide new values flutter/flutter#112038

Closed

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add static code diagnostic consistent-update-render-object #1004

feat: add static code diagnostic consistent-update-render-object #1004

incendial commented Sep 18, 2022

incendial commented Sep 18, 2022

incendial commented Sep 18, 2022

fzyzcjy commented Sep 18, 2022 •

edited

fzyzcjy commented Sep 18, 2022

incendial commented Sep 18, 2022

fzyzcjy commented Sep 18, 2022

github-actions bot commented Sep 19, 2022 •

edited

github-actions bot commented Sep 19, 2022 •

edited

codecov bot commented Sep 19, 2022 •

edited

fzyzcjy commented Sep 19, 2022 •

edited

incendial commented Sep 20, 2022

fzyzcjy commented Sep 20, 2022

incendial commented Sep 20, 2022 •

edited

incendial commented Sep 20, 2022

incendial commented Sep 20, 2022

sonarcloud bot commented Sep 20, 2022

fzyzcjy commented Sep 20, 2022

feat: add static code diagnostic consistent-update-render-object #1004

feat: add static code diagnostic consistent-update-render-object #1004

Conversation

incendial commented Sep 18, 2022

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

incendial commented Sep 18, 2022

incendial commented Sep 18, 2022

fzyzcjy commented Sep 18, 2022 • edited

fzyzcjy commented Sep 18, 2022

incendial commented Sep 18, 2022

fzyzcjy commented Sep 18, 2022

github-actions bot commented Sep 19, 2022 • edited

Dart Code Metrics unused files report of dart_code_metrics. ✅

Summary

github-actions bot commented Sep 19, 2022 • edited

Dart Code Metrics analyze report of dart_code_metrics. ✅

Summary

codecov bot commented Sep 19, 2022 • edited

Codecov Report

fzyzcjy commented Sep 19, 2022 • edited

incendial commented Sep 20, 2022

fzyzcjy commented Sep 20, 2022

incendial commented Sep 20, 2022 • edited

incendial commented Sep 20, 2022

incendial commented Sep 20, 2022

sonarcloud bot commented Sep 20, 2022

fzyzcjy commented Sep 20, 2022

fzyzcjy commented Sep 18, 2022 •

edited

github-actions bot commented Sep 19, 2022 •

edited

github-actions bot commented Sep 19, 2022 •

edited

codecov bot commented Sep 19, 2022 •

edited

fzyzcjy commented Sep 19, 2022 •

edited

incendial commented Sep 20, 2022 •

edited