language
language copied to clipboard
Remove the colon-syntax for default values.
For historical reasons, named optional parameters can specify their default value using either :
or =
.
The =
was introduced later because it's better. It's probably about time to remove the ability to use :
.
So, we should do so for Dart 3.0.
Example:
int someInt({int x: 0}) => x;
is currently valid, will become invalid.
We have a lint recommending using =
, and it's in our recommended set, so there should hopefully be very few remaining uses, most likely only in code so old it's not null-safe anyway.
Might also open up the syntax for something else, even if I don't know what. (Types on the right, anybody?)
@bwilkerson does dart fix
already update these from using :
to using =
? If not, how much would work that be to support?
We may wish to start emitting a warning in the next release that this is going away.
Sorry I missed the ping.
dart fix
will currently only fix these is the lint prefer_equal_for_default_values
is enabled.
Sounds like we should convert the lint to a warning so that it's always enabled.
I think the right path forward is to mark the lint as deprecated when the warning is implemented. Doing so will enable a fix to remove the lint from the analysis options file.
dartfmt
was the tool that did this fix IIRC
Sounds like we should convert the lint to a warning so that it's always enabled.
Yes, I think a roll-out plan like this might work:
- Current: flagged by lint
prefer_equal_for_default_values
, which is part oflints/recommended.yaml
- Dart 2.19: make it a warning in the analyzer; deprecate the lint
- Dart 3: make it an error generally, remove the lint
@bwilkerson does that sound right?
And if we do so, would a developer using 2.19 who for some reason want to ignore the warning be able to suppress it with an ignore
in their analysis config?
That sounds right to me. I assume that we'll put out a breaking change notice before 2.19 and again before 3.0.
And if we do so, would a developer using 2.19 who for some reason want to ignore the warning be able to suppress it with an ignore in their analysis config?
Yes. Warnings can be ignored via the analysis options file or by using an ignore comment.
I have a CL to add the warning and am using it to gauge the amount of work that will be required to land it. I don't know what the time frame for 2.19 looks like, so I don't know when this needs to land.
Thanks! 2.19 branching is several months away.
I noticed that we have a lot of occurrences in the platform libraries (which includes some of the oldest Dart code in existence, so not entirely surprising). Some clean-up will definitely be needed.
Most can be handled by a dart format --fix-named-default-separator
run, but a few are hiding in strings used to generate the code.
(https://dart-review.googlesource.com/c/sdk/+/254700)
I noticed the same when I ran a dry-run on the CL that adds the analyzer support (https://dart-review.googlesource.com/c/sdk/+/254467).
As can be seen from the CBuild failure on https://dart-review.googlesource.com/c/sdk/+/254467, there are still a fair number of uses of colon-syntax internally. We probably want to get the lint enabled internally as a path toward being able to land this change.
This would probably be useful to get the code migrated: https://github.com/dart-lang/sdk/issues/47219
@bwilkerson we've been doing a ton of cleanup. Can you try to run your CL checks again after rebasing?
I'm not sure the timeline for the analyzer warning, but perhaps prefer_equal_for_default_values
should be moved to the core
lints set rather than recommended
? At least if the next release of lints
is intended before that warning.
Great suggestion, @parlough !!
I like that in spirit, but I think practically only pretty old Dart code is likely to use this style, and I doubt any of that is a) configured to use our new lints, and b) would upgrade to a new version of those lints. So I speculate that it would make no practical difference?
The analyzer changes have now landed.
@bwilkerson can I get you to create a second changelist for the 3.0 alpha change, which makes this an error and removes the lint? This CL would be landed right after we've done the last branch point for 2.19 and the main branch is open for 3.0 development.
Perhaps the Dart 3 alpha change we can split into two steps: a) make it an error, and b) remove the lint. I think the former is the most urgent.
... can I get you to create a second changelist for the 3.0 alpha change ...
Sorry I missed the ping again. Yes.
Perhaps the Dart 3 alpha change we can split into two steps: a) make it an error, and b) remove the lint.
Yes, it will likely be two steps. We're trying to deprecate the lint before 2.19 so that users will know that it's no longer needed and will be removed.
@bwilkerson any updates on the CL to make this an error? I don't see it here: https://dart-review.git.corp.google.com/q/message:%2522%255B3.0+alpha%255D%2522
It should happen automatically with the change in the SDK version number. There are changes in that CL to fix some tests that were reporting the warning to make them expect the error, so I assumed that everything was working correctly. To the best of my knowledge, there is no further work to be done and we can close this issue once that CL has landed.
Indeed that is the case 🥳 :
$ dart --version
Dart SDK version: 3.0.0-edge.606a64a743b80e5f21d7818e0d4119376083e1d1 (be) (Tue Dec 6 04:04:23 2022 +0000) on "macos_arm64"
$ cat bin/colons.dart
import 'package:colons/colons.dart' as colons;
int someInt({int x: 0}) => x;
$ dart analyze .
Analyzing .... 0.7s
error • bin/colons.dart:3:19 • Using a colon as a separator before a default value is no longer supported. Try replacing the colon with an equal sign. •
obsolete_colon_for_default_value
1 issue found.
Thanks!
@lrhn - the analyzer reports this as an error now, but the CFE does not. Shouldn't the CFE as well?
If so, can you please open an issue on the CFE for Dart 3?
FYI - @mit-mit @itsjustkevin
The CFE issue is here.
Echoing a comment there on the main issue here for posterity: This is a language versioned changed. It's an error to use :
as a default value separator in a Dart 3.0 library, but not in a library whose language version is <3.0.
The CFE change handled in https://github.com/dart-lang/sdk/commit/bd02ce7a7f16408b99d04fee2d758e1fbe96dabe.
I'm not aware of any remaining work. @munificent can we go ahead and close this given all the changes are in main?
I think so. This change is @lrhn's baby, but as far as I know, it's done.
Closing as there is no known work