dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.23k stars 1.57k forks source link

dart fix for prefer_final_parameters no longer works from command line. #54499

Open mrcsh opened 10 months ago

mrcsh commented 10 months ago

I think this regressed in the last major release.

We use prefer_final_parameters in our analysis_options.yaml file and until recently dart fix will add the final key word when run from the command line.
The quick fixes from both Android Studio and vscode continue to work or I would have noticed it sooner.

We use hive and we rely on dart fix --apply to fix-up the warnings the generated adapter code, that is not working now.

Using dart --version Dart SDK version: 3.2.3 (stable) (Tue Dec 5 17:58:33 2023 +0000) on "linux_x64"

``` dart analyze lib Analyzing lib... 2.4s info • model/chat_message.g.dart:14:20 • The parameter 'reader' should be final. Try making the parameter final. • prefer_final_parameters info • model/chat_message.g.dart:41:14 • The parameter 'writer' should be final. Try making the parameter final. • prefer_final_parameters info • model/chat_message.g.dart:41:35 • The parameter 'obj' should be final. Try making the parameter final. • prefer_final_parameters info • model/chat_message.g.dart:84:20 • The parameter 'other' should be final. Try making the parameter final. • prefer_final_parameters info • model/chat_message.g.dart:96:19 • The parameter 'reader' should be final. Try making the parameter final. • prefer_final_parameters info • model/chat_message.g.dart:112:14 • The parameter 'writer' should be final. Try making the parameter final. • prefer_final_parameters info • model/chat_message.g.dart:112:35 • The parameter 'obj' should be final. Try making the parameter final. • prefer_final_parameters info • model/chat_message.g.dart:133:20 • The parameter 'other' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_asset.g.dart:14:16 • The parameter 'reader' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_asset.g.dart:33:14 • The parameter 'writer' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_asset.g.dart:33:35 • The parameter 'obj' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_asset.g.dart:60:20 • The parameter 'other' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_asset.g.dart:72:14 • The parameter 'reader' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_asset.g.dart:92:14 • The parameter 'writer' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_asset.g.dart:92:35 • The parameter 'obj' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_asset.g.dart:119:20 • The parameter 'other' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_device.g.dart:14:17 • The parameter 'reader' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_device.g.dart:38:14 • The parameter 'writer' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_device.g.dart:38:35 • The parameter 'obj' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_device.g.dart:75:20 • The parameter 'other' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_entity.g.dart:14:17 • The parameter 'reader' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_entity.g.dart:24:46 • The parameter 'k' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_entity.g.dart:24:57 • The parameter 'v' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_entity.g.dart:30:14 • The parameter 'writer' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_entity.g.dart:30:35 • The parameter 'obj' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_entity.g.dart:49:20 • The parameter 'other' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_entity_base.g.dart:14:21 • The parameter 'reader' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_entity_base.g.dart:28:14 • The parameter 'writer' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_entity_base.g.dart:28:35 • The parameter 'obj' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_entity_base.g.dart:45:20 • The parameter 'other' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_group.g.dart:14:16 • The parameter 'reader' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_group.g.dart:31:14 • The parameter 'writer' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_group.g.dart:31:35 • The parameter 'obj' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_group.g.dart:54:20 • The parameter 'other' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_group_list.g.dart:14:20 • The parameter 'reader' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_group_list.g.dart:27:14 • The parameter 'writer' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_group_list.g.dart:27:35 • The parameter 'obj' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_group_list.g.dart:42:20 • The parameter 'other' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_msg_reaction.g.dart:14:26 • The parameter 'reader' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_msg_reaction.g.dart:27:14 • The parameter 'writer' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_msg_reaction.g.dart:27:35 • The parameter 'obj' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_msg_reaction.g.dart:42:20 • The parameter 'other' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_org.g.dart:14:14 • The parameter 'reader' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_org.g.dart:33:14 • The parameter 'writer' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_org.g.dart:33:35 • The parameter 'obj' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_org.g.dart:60:20 • The parameter 'other' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_pending.g.dart:14:18 • The parameter 'reader' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_pending.g.dart:31:14 • The parameter 'writer' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_pending.g.dart:31:35 • The parameter 'obj' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_pending.g.dart:54:20 • The parameter 'other' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_relation.g.dart:14:20 • The parameter 'reader' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_relation.g.dart:25:14 • The parameter 'writer' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_relation.g.dart:25:35 • The parameter 'obj' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_relation.g.dart:36:20 • The parameter 'other' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_remote.g.dart:14:23 • The parameter 'reader' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_remote.g.dart:30:14 • The parameter 'writer' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_remote.g.dart:30:35 • The parameter 'obj' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_remote.g.dart:51:20 • The parameter 'other' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_unified.g.dart:14:28 • The parameter 'reader' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_unified.g.dart:21:46 • The parameter 'k' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_unified.g.dart:21:57 • The parameter 'v' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_unified.g.dart:27:14 • The parameter 'writer' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_unified.g.dart:27:35 • The parameter 'obj' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_unified.g.dart:40:20 • The parameter 'other' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_user.g.dart:14:15 • The parameter 'reader' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_user.g.dart:33:14 • The parameter 'writer' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_user.g.dart:33:35 • The parameter 'obj' should be final. Try making the parameter final. • prefer_final_parameters info • model/db_user.g.dart:60:20 • The parameter 'other' should be final. Try making the parameter final. • prefer_final_parameters 68 issues found. dart fix --apply lib Computing fixes in lib... 15.7s Nothing to fix! ```
parlough commented 10 months ago

It seems this is because the bulk fix processor underlying dart fix explicitly skips generated files and has since https://github.com/dart-lang/sdk/commit/adcef136a69aa36fca0236ac83b2515d847a6901.

This often makes sense as otherwise the generation output will differ from what you have in your source repository, potentially making it unclear if the generation output itself changed.

Most cases where lints are triggered in a generated file are solved by either updating the generator or adding an ignore_for_file comment at the top of the file during generation. Many packages support some form of this ignoring functionality, including ones that use package:source_gen (https://pub.dev/packages/source_gen#configuring-combining_builder) through a custom build config option.

@bwilkerson May have some other insight as he implemented the original change to skip generated files.

I will say, perhaps it would make sense for dart fix to still work on generated files if the specific file is passed, since that means you're explicitly choosing to do so? Or a flag to enable fixes on generated files?

bwilkerson commented 10 months ago

I don't remember the impetus for that change, but the reasoning is that any changes that dart fix makes will be lost the next time the generator is run, making it "better" (in some sense) to fix the generator.

I don't think we'd considered the use case of using dart fix as a post-processing step as part of the generation process.

I'm not sure what I think of that idea in general, but marking parameters as being final in generated code does seem fairly pointless; the purpose of the lint is to keep developers from assigning to a parameter, but presumably users aren't hand editing the generated files and the generator won't be impacted if they are marked final after the generator has finished running. Maybe I'm missing a benefit that you're thinking of.

mrcsh commented 10 months ago

Well the benefit is I don't have many dozens of warnings in many files I have to go and manually fix.

If dart is not going fix generated code it should not report any warnings on the same.

Does it make any real difference if the parameters a final, not likely since as you say as the code should not be manually edited. But it's impossible to get to zero warnings with out pain as things are now.
In general I am not a fan of suppressing warnings, but I can sed the files if necessary as a post processing step or rename them so the are not treated as generated.

They are never checked in "unfixed" so I definitely do not agree with the logic behind not fixing them.

But since it appears to be intentional I can workaround it.

mrcsh commented 10 months ago

I am seeing some unexpected behavior. I tried renaming the files and removed the // GENERATED CODE comment and it sill did not fix up the file.

I manually removed the final from a parameter in non-generated code to see and ran it again and it fixed both the one I removed in addition to the ones in the generated code.
But it won't fix the included code for the other files until it has to fix also something in the including file.

bwilkerson commented 10 months ago

Well the benefit is I don't have many dozens of warnings in many files I have to go and manually fix.

That's fair.

If dart is not going fix generated code it should not report any warnings on the same.

I agree with you. Generated code should be handled consistently.

I don't think there's any decision yet, though, about how to treat generated code. We've had feedback from users both in favor of and apposed to generating warnings.

They are never checked in "unfixed" so I definitely do not agree with the logic behind not fixing them.

That's interesting. I wonder whether the difference in opinion (among users in general concerning warnings) is related to whether or not generated code is committed to the repository.

But since it appears to be intentional I can workaround it.

It was definitely intentional. Whether it was the right choice is a matter for debate.

I manually removed the final from a parameter in non-generated code to see and ran it again and it fixed both the one I removed in addition to the ones in the generated code.

That's definitely a bug. Whether or not fixes are applied to one file should not depend on whether fixes are applied to another file.

@keertip

mrcsh commented 10 months ago

We pretty much check in all our generated code especially when it takes a while to generate. We only stopped checking in some generated code in our go projects because the macos compiled generator produced slightly different results (different white space) than the Linux version and the ping-ponging was super annoying. But with most of our use cases the code generators are not run very often and only as needed.

Personally I would vote for no special treatment of generated code, even if its gated by an command line option.

Should I file a bug for the weird fix behavior?

bwilkerson commented 10 months ago

This issue will probably suffice, but thanks for the offer.