New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Lint when member
might need to be this.member
in constructor bodies
#2552
Comments
(Edit: Ignore, I'm the one who was confused.) |
In order to prevent false positives we would need to be able to statically determine when the user intended to refer to the field rather than the parameter, and I'm not sure that's feasible. But we could have a style lint that disallowed using the name of a field as a parameter (with the obvious exception for field formal parameters). It might be useful to run some analysis of existing code to find out how often there are non-field formal parameters whose name is the same as a field from the enclosing class (or an inherited field?). |
The lint probably doesn't need to be perfect and catch all cases; catching common cases would be an improvement. |
* Fix an implicit cast from `BarRendererDecorator<Object>` to `BarRendererDecorator<D>`. `BarRenderer<D>.barRendererDecorator` should be parameterized on `Object` instead of `D` because it is initialized from a `BarRendererConfig<Object>`. * Fix some code in `SelectionModel.fromConfig` that called `Iterable<E>.contains` with a completely unrelated type, meaning that `contains` would always return false. * Fix some analysis errors in tests. * Add missing `@override` annotations. * Add some missing type parameters to generics. (This is not comprehensive.) * Fix some implicit casts. (This is not comprehensive.) Also, fix a bug that I introduced in my earlier change to `A11yExploreBehavior`'s constructor. When I moved the default value for `exploreModeTrigger` from the parameter list to the initializer list, the constructor body still referenced the argument (which now had a default value of `null`) instead of the initialized member. (I've filed dart-lang/linter#2552 to lint such cases in the future.) PiperOrigin-RevId: 367157475
* Fix an implicit cast from `BarRendererDecorator<Object>` to `BarRendererDecorator<D>`. `BarRenderer<D>.barRendererDecorator` should be parameterized on `Object` instead of `D` because it is initialized from a `BarRendererConfig<Object>`. * Fix some code in `SelectionModel.fromConfig` that called `Iterable<E>.contains` with a completely unrelated type, meaning that `contains` would always return false. * Fix some analysis errors in tests. * Add missing `@override` annotations. * Add some missing type parameters to generics. (This is not comprehensive.) * Fix some implicit casts. (This is not comprehensive.) Also, fix a bug that I introduced in my earlier change to `A11yExploreBehavior`'s constructor. When I moved the default value for `exploreModeTrigger` from the parameter list to the initializer list, the constructor body still referenced the argument (which now had a default value of `null`) instead of the initialized member. (I've filed dart-lang/linter#2552 to lint such cases in the future.) PiperOrigin-RevId: 367157475
* Fix an implicit cast from `BarRendererDecorator<Object>` to `BarRendererDecorator<D>`. `BarRenderer<D>.barRendererDecorator` should be parameterized on `Object` instead of `D` because it is initialized from a `BarRendererConfig<Object>`. * Fix some code in `SelectionModel.fromConfig` that called `Iterable<E>.contains` with a completely unrelated type, meaning that `contains` would always return false. * Fix some analysis errors in tests. * Add missing `@override` annotations. * Add some missing type parameters to generics. (This is not comprehensive.) * Fix some implicit casts. (This is not comprehensive.) Also, fix a bug that I introduced in my earlier change to `A11yExploreBehavior`'s constructor. When I moved the default value for `exploreModeTrigger` from the parameter list to the initializer list, the constructor body still referenced the argument (which now had a default value of `null`) instead of the initialized member. (I've filed dart-lang/linter#2552 to lint such cases in the future.) PiperOrigin-RevId: 367157475
* Fix an implicit cast from `BarRendererDecorator<Object>` to `BarRendererDecorator<D>`. `BarRenderer<D>.barRendererDecorator` should be parameterized on `Object` instead of `D` because it is initialized from a `BarRendererConfig<Object>`. * Fix some code in `SelectionModel.fromConfig` that called `Iterable<E>.contains` with a completely unrelated type, meaning that `contains` would always return false. * Fix some analysis errors in tests. * Add missing `@override` annotations. * Add some missing type parameters to generics. (This is not comprehensive.) * Fix some implicit casts. (This is not comprehensive.) Also, fix a bug that I introduced in my earlier change to `A11yExploreBehavior`'s constructor. When I moved the default value for `exploreModeTrigger` from the parameter list to the initializer list, the constructor body still referenced the argument (which now had a default value of `null`) instead of the initialized member. (I've filed dart-lang/linter#2552 to lint such cases in the future.) PiperOrigin-RevId: 367157475
* Remove unnecessary (and harmful) explicit types Fix a number of cases where explicit types neglected to specify type parameters for generics, causing them to *lose* type information and to be inadvertently parameterized on `dynamic`. In these cases, it's better to omit the explicit types and let the type be inferred. PiperOrigin-RevId: 362608115 * Fix some forEach usage * `.forEach((x) => foo(x))` => `.forEach(foo)`. (Fixes some violations of the Dart lint unnecessary_lambdas.) * Rewrite a `.forEach` site that could benefit from an early exit. * Rewrite some .`forEach` sites that did repeated string concatenations (which I believe is O(n^2)) to use `List<String>.join()` instead. * Rewrite one site to avoid depending on nullable values. PiperOrigin-RevId: 362608336 * Use .of() instead of .from() when possible Use `List.of()`/`Set.of()`/etc. instead of the `.from()` constructors when possible to avoid losing type information. PiperOrigin-RevId: 362608422 * Fix violations of implicit-dynamic (part 1) Fix some unintended uses of `dynamic` due to missing types. This is a first pass at cleaning up `implicit-dynamic` violations. PiperOrigin-RevId: 362608551 * charts_common: Fix violations of implicit-dynamic (part 2) * Parameterize some types on `Object`. Prefer `Object` to `dynamic` because it's more explicit and to allow distinguishing between nullable and non-nullable types later. * Use `AttributeKey<Generic<Object>>` instead of `AttributeKey<Generic>`. (These ideally would be `AttributeKey<Generic<D>>`, but these are global constants.) * Refactor some code in `range_annotation.dart` to avoid `dynamic` usage. PiperOrigin-RevId: 362625826 * Fix violations of implicit-dynamic (part 3) Add more explicit type parameters for generics. PiperOrigin-RevId: 362625972 * Fix violations of implicit-dynamic (part 4) Add more explicit type parameters for generics. PiperOrigin-RevId: 362628251 * Fix violations of implicit-dynamic (part 5) Add more explicit type parameters for generics. Also fix some code in `Slider<D>._updateViewData` that assumed that the domain was a `num` (and only happened to work if `D` was `dynamic`). PiperOrigin-RevId: 362630599 * Fix violations of implicit-dynamic (part 6) Add more explicit type parameters for generics. PiperOrigin-RevId: 362632113 * Fix violations of implicit-dynamic (part 7) Fix remaining `implicit-dynamic` sites by using `dynamic` and `Object` explicitly. PiperOrigin-RevId: 362632180 * Clean up implicit downcasts Clean up implicit downcasts by: * Adding explicit casts. * Changing `GenericClass` to `GenericClass<T>` in a few more places to avoid implicitly using `dynamic`. * Removing explicit `GenericClass` declarations in other places so that the type is inferred instead of being retyped to `GenericClass<dynamic>`. * Changing `Derived object = baseObject;` to `var object = baseObject as Derived`. * Adding lots of `.toDouble()` calls to fix implicit casts from `num`. Some of these sites maybe could use `as double` instead, but using `.toDouble()` is safer and easier to reason about. PiperOrigin-RevId: 363460124 * Clean up violations of Dart lint omit_local_variable_types PiperOrigin-RevId: 363460354 * Remove unnecessary null-aware operators when computing hash codes A number of `.hashCode` implementations had computations of the form: ```dart hash = hash * k + nullableField?.hashCode ?? 0; ``` but this is wrong because `??` has lower precedence than `+`, so the code is equivalent to: ```dart hash = (hash * k + nullableField?.hashCode) ?? 0; ``` instead of the intended: ```dart hash = hash * k + (nullableField?.hashCode ?? 0); ``` The use of `?.` and `??` aren't necessary anyway; all Dart objects implement `.hashCode`, including `null`. PiperOrigin-RevId: 363470863 * Simplify and strengthen TimeStepper assertions All of the `TimeStepper` implementations individually assert that `allowedTickIncrements` is non-empty. Strengthen that assertion by: * Documenting in the `TimeStepper.allowedTickIncrements` interface that it is expected to be non-empty. * Moving `assert(allowedTickIncrements.isNotEmpty)` checks from the constructors of the various implementations into the `BaseTimeStepper` constructor. While I'm touching this, also simplify all assertions of the form: ```dart // All increments must be P. assert(allowedTickIncrements.any((increment) => !P) == false); ``` to: ```dart assert(allowedTickIncrements.every((increment) => P)); ``` PiperOrigin-RevId: 363471428 * Make A11yExploreBehavior(exploreModeTrigger: null) use the default `DomainA11yExploreBehavior` derives from `A11yExploreBehavior` and passes through its construction arguments. However, because it did not duplicate `A11yExploreBehavior`'s default argument for the optional `exploreModeTrigger` parameter, omitting it passed `null` instead of using the base class's intended default value. Make the base class constructor use the default value whenever `explorModeTrigger` is `null`. PiperOrigin-RevId: 363568651 * Remove broken NumericTickProvider.getTicks caching NumericTickProvider.getTicks has vestigal code that attempted to avoid recalculating ticks by checking if the tick parameters have changed. However, its check is always true, and therefore it always recalculates ticks. I attempted to fix the caching, but that broke screenshot tests in charts_web. For now, remove the caching entirely. PiperOrigin-RevId: 363568737 * Expose BasicDateTimeTickFormatterSpec in charts_flutter PiperOrigin-RevId: 364455090 * Internal changes PiperOrigin-RevId: 365704793 * Fix more analysis errors * Fix an implicit cast from `BarRendererDecorator<Object>` to `BarRendererDecorator<D>`. `BarRenderer<D>.barRendererDecorator` should be parameterized on `Object` instead of `D` because it is initialized from a `BarRendererConfig<Object>`. * Fix some code in `SelectionModel.fromConfig` that called `Iterable<E>.contains` with a completely unrelated type, meaning that `contains` would always return false. * Fix some analysis errors in tests. * Add missing `@override` annotations. * Add some missing type parameters to generics. (This is not comprehensive.) * Fix some implicit casts. (This is not comprehensive.) Also, fix a bug that I introduced in my earlier change to `A11yExploreBehavior`'s constructor. When I moved the default value for `exploreModeTrigger` from the parameter list to the initializer list, the constructor body still referenced the argument (which now had a default value of `null`) instead of the initialized member. (I've filed dart-lang/linter#2552 to lint such cases in the future.) PiperOrigin-RevId: 367157475 * Make better use of string interpolation * Replace string concatenation with `+` with string interpolation in some cases I noticed. * Replace `${variable.toString()}` in interpolated strings with just `$variable`. * Remove some unnecessary braces. These change will reduce to the need to use the null-assertion operator (`!`) in an upcoming change to enable null-safety. PiperOrigin-RevId: 367157594 * Make more named parameters required (and make some not required) Annotate named parameters with `@required` in cases where they're actually required (i.e., the callee unconditionally dereferences the argument) and in some cases where all callers already supply arguments. This will avoid needing to make some members nullable when null-safety is enabled. I also removed `@required` from `DateTimeTickFormatter.format`'s `stepSize` parameter since it's not required by the base `TickFormatter.format` method and is therefore unenforceable. Additionally, `LayoutViewConfig`'s constructor used `@required` for its arguments, but almost none of its derived classes supply those arguments. (That these don't generate analysis warnings is a known deficiency of the analyzer when derived classes rely on implicit calls to `super()`.) Even though `LayoutViewConfig` was intended to have required parameters, I instead made them optional to preserve existing behavior and because I did not know what values to pass for each derived class. Finally, initialize some data in tests and add some mock responses that they will no longer return null. This is in preparation for making some types non-nullable. PiperOrigin-RevId: 367160939 * Prefer initializer lists to constructor bodies Initialize class members in initializer lists instead of in constructor bodies when possible: * Initializer lists avoid accidental name shadowing bugs where `member` is use instead of `this.member`. * Initializer lists allow more members to be declared as `final`. In turn, this will avoid needing to declare those members as nullable or as `late` when null-safety is enabled. PiperOrigin-RevId: 367161613 * Add and use construction parameters instead of mutating members Replace code of the form: ```dart var foo = Foo() ..property1 = value1 ..property2 = value2 ..property3 = value3; ``` with ```dart var foo = Foo( property1: value1, property2: value2, property3: value3, ); ``` so that the members can be `final` when possible. This also will allow members to be non-nullable when we enable null-safety. PiperOrigin-RevId: 367161773 * Add a NullablePoint class and use it instead of Point<double> charts_common expects to be able use null coordinate values, but when null-safety is enabled, this will no longer possible with `dart:math`'s `Point` class. Add a `NullablePoint` class to use as a substitute. This might not be the ideal long-term solution, but it should preserve existing behavior for now. I also have replaced only the sites that clearly depended on null coordinates; it is quite possible that there are sites not covered by automated tests that still need to switch from `Point<double>` to `NullablePoint`. PiperOrigin-RevId: 367165943 * Make better use of null-aware operators PiperOrigin-RevId: 367539685 * Explicitly handle null values in more places * Add explicit handling of `null` in cases where there is an obvious way to fail gracefully. * Add some default values/arguments. * Make some functions avoid returning `null`. In some cases I just added `assert`s. Some of these changes aren't strictly necessary since the code apparently works already. However, when null-safety is enabled and types are made explicitly nullable, I think that this is preferable over greater usage of null-assertion operators (`!`). PiperOrigin-RevId: 367542367 * Miscellaneous, non-trivial changes * Replace some `Iterable.forEach` loops with `for-in` loops: * `return`s in `forEach` can be misleading, and `for-in` with `continue` better expresses intent. * Closures cannot use promoted types from the parent environment. When null-safety is enabled, using `forEach` therefore would require unnecessary use of the the null-assertion operator (`!`). * Adjust some comments. * Simplify some `sort` callbacks to use `Comparable.compareTo` instead of performing manual comparisons. * Use redirecting constructors in a couple of places. * Simplify `Timer` cancellation. * Remove an unused parameter. * Other miscellaneous cleanup. PiperOrigin-RevId: 367542429 * Disable null-safety for tests Explicitly disable null-safety for files in `test/` in preparation for enabling it for `lib/`. PiperOrigin-RevId: 367542475 * Mechanical null migration changes Add `?` to many types, add null assertion operators (`!`) in a lot of places, add `late` keywords, and change `@required` to `required` everywhere. To reduce the number of sites that need null assertions, I shadowed member variables with identically named local variables so that a single null assertion will cause type promotion. That is, intead of: ```dart f(member!.x + member!.y + member!.z); ``` When possible, I preferred: ```dart final member = this.member!; f(member.x + member.y + member.z); ``` PiperOrigin-RevId: 367542574 * Roll back null safety migration change Enabling null safety broke some internal tests. PiperOrigin-RevId: 367584842 * Reapply null safety migration change PiperOrigin-RevId: 368179137 * Post-null-migration cleanup * Dart 2.12 allows type promotion to occur from `as` casts, so take advantage of that. This will fix the `unnecessary_cast` violations. * I also noticed that `OrdinalScale`'s constructor has an optional `tickDrawStrategy` parameter but does nothing with it. Fix the constructor to use it to initialize its `tickDrawStrategy` member. * Change another `.forEach` loop to `for-in` so that it can stop iterating early when possible. * It's logically impossible for `RangeAnnotation<D>._annotationMap` to store null values, so checking `_annotationMap.containsKey(key)` is equivalent to checking if `_annotationMap[key]` is non-null, and the latter is slightly simpler. PiperOrigin-RevId: 368324239 * Internal changes PiperOrigin-RevId: 368444143 * Fix build warnings in charts_common PiperOrigin-RevId: 370588847 * Automated g4 rollback of changelist 370588847. WANT_LGTM=any *** Reason for rollback *** Breaks multiple Greentea FE widgets, causing presubmit TAP failures. *** Original change description *** Fix build warnings in charts_common *** PiperOrigin-RevId: 370712865 * Automated g4 rollback of changelist 370712865. *** Reason for rollback *** Rollforward after fix in greentea submitted in cl/370722339. Global presubmit is showing all targets ok after deflakes *** Original change description *** Automated g4 rollback of changelist 370588847. WANT_LGTM=any *** Reason for rollback *** Breaks multiple Greentea FE widgets, causing presubmit TAP failures. *** Original change description *** Fix build warnings in charts_common *** *** PiperOrigin-RevId: 371141851 * Internal changes PiperOrigin-RevId: 371425261 * Internal changes PiperOrigin-RevId: 371617443 * Internal changes PiperOrigin-RevId: 371908750 * Internal changes PiperOrigin-RevId: 372183756 * Internal changes PiperOrigin-RevId: 372426281 * Internal changes PiperOrigin-RevId: 374314810 * Internal changes PiperOrigin-RevId: 375978148 * Internal changes PiperOrigin-RevId: 375997369 * Internal changes PiperOrigin-RevId: 378428877 * Internal changes PiperOrigin-RevId: 378497037 * Migrate deprecated TextTheme API from body1 to bodyText2 This deprecated API is scheduled to be removed in flutter/flutter#83924 PiperOrigin-RevId: 378523673 * Internal changes PiperOrigin-RevId: 379206830 * Internal changes PiperOrigin-RevId: 379784071 * Internal changes PiperOrigin-RevId: 379986913 * Fixed nullability issues in chart classes. PiperOrigin-RevId: 380237923 * Migrate //third_party/dart/charts_flutter to null safety. PiperOrigin-RevId: 381309874 * Migrate //third_party/dart/charts_flutter/examples to null safety. PiperOrigin-RevId: 381509039 * Bump sdk version for nullability support to hopefully get travis tests to pass PiperOrigin-RevId: 381579690 * Fix mockito testing for null safety PiperOrigin-RevId: 382373361 * Fixing tests removing some mockito use PiperOrigin-RevId: 382380502 Co-authored-by: jamesdlin <jamesdlin@google.com> Co-authored-by: Googler <noreply@google.com> Co-authored-by: nolankelly <nolankelly@google.com>
* Remove unnecessary (and harmful) explicit types Fix a number of cases where explicit types neglected to specify type parameters for generics, causing them to *lose* type information and to be inadvertently parameterized on `dynamic`. In these cases, it's better to omit the explicit types and let the type be inferred. PiperOrigin-RevId: 362608115 * Fix some forEach usage * `.forEach((x) => foo(x))` => `.forEach(foo)`. (Fixes some violations of the Dart lint unnecessary_lambdas.) * Rewrite a `.forEach` site that could benefit from an early exit. * Rewrite some .`forEach` sites that did repeated string concatenations (which I believe is O(n^2)) to use `List<String>.join()` instead. * Rewrite one site to avoid depending on nullable values. PiperOrigin-RevId: 362608336 * Use .of() instead of .from() when possible Use `List.of()`/`Set.of()`/etc. instead of the `.from()` constructors when possible to avoid losing type information. PiperOrigin-RevId: 362608422 * Fix violations of implicit-dynamic (part 1) Fix some unintended uses of `dynamic` due to missing types. This is a first pass at cleaning up `implicit-dynamic` violations. PiperOrigin-RevId: 362608551 * charts_common: Fix violations of implicit-dynamic (part 2) * Parameterize some types on `Object`. Prefer `Object` to `dynamic` because it's more explicit and to allow distinguishing between nullable and non-nullable types later. * Use `AttributeKey<Generic<Object>>` instead of `AttributeKey<Generic>`. (These ideally would be `AttributeKey<Generic<D>>`, but these are global constants.) * Refactor some code in `range_annotation.dart` to avoid `dynamic` usage. PiperOrigin-RevId: 362625826 * Fix violations of implicit-dynamic (part 3) Add more explicit type parameters for generics. PiperOrigin-RevId: 362625972 * Fix violations of implicit-dynamic (part 4) Add more explicit type parameters for generics. PiperOrigin-RevId: 362628251 * Fix violations of implicit-dynamic (part 5) Add more explicit type parameters for generics. Also fix some code in `Slider<D>._updateViewData` that assumed that the domain was a `num` (and only happened to work if `D` was `dynamic`). PiperOrigin-RevId: 362630599 * Fix violations of implicit-dynamic (part 6) Add more explicit type parameters for generics. PiperOrigin-RevId: 362632113 * Fix violations of implicit-dynamic (part 7) Fix remaining `implicit-dynamic` sites by using `dynamic` and `Object` explicitly. PiperOrigin-RevId: 362632180 * Clean up implicit downcasts Clean up implicit downcasts by: * Adding explicit casts. * Changing `GenericClass` to `GenericClass<T>` in a few more places to avoid implicitly using `dynamic`. * Removing explicit `GenericClass` declarations in other places so that the type is inferred instead of being retyped to `GenericClass<dynamic>`. * Changing `Derived object = baseObject;` to `var object = baseObject as Derived`. * Adding lots of `.toDouble()` calls to fix implicit casts from `num`. Some of these sites maybe could use `as double` instead, but using `.toDouble()` is safer and easier to reason about. PiperOrigin-RevId: 363460124 * Clean up violations of Dart lint omit_local_variable_types PiperOrigin-RevId: 363460354 * Remove unnecessary null-aware operators when computing hash codes A number of `.hashCode` implementations had computations of the form: ```dart hash = hash * k + nullableField?.hashCode ?? 0; ``` but this is wrong because `??` has lower precedence than `+`, so the code is equivalent to: ```dart hash = (hash * k + nullableField?.hashCode) ?? 0; ``` instead of the intended: ```dart hash = hash * k + (nullableField?.hashCode ?? 0); ``` The use of `?.` and `??` aren't necessary anyway; all Dart objects implement `.hashCode`, including `null`. PiperOrigin-RevId: 363470863 * Simplify and strengthen TimeStepper assertions All of the `TimeStepper` implementations individually assert that `allowedTickIncrements` is non-empty. Strengthen that assertion by: * Documenting in the `TimeStepper.allowedTickIncrements` interface that it is expected to be non-empty. * Moving `assert(allowedTickIncrements.isNotEmpty)` checks from the constructors of the various implementations into the `BaseTimeStepper` constructor. While I'm touching this, also simplify all assertions of the form: ```dart // All increments must be P. assert(allowedTickIncrements.any((increment) => !P) == false); ``` to: ```dart assert(allowedTickIncrements.every((increment) => P)); ``` PiperOrigin-RevId: 363471428 * Make A11yExploreBehavior(exploreModeTrigger: null) use the default `DomainA11yExploreBehavior` derives from `A11yExploreBehavior` and passes through its construction arguments. However, because it did not duplicate `A11yExploreBehavior`'s default argument for the optional `exploreModeTrigger` parameter, omitting it passed `null` instead of using the base class's intended default value. Make the base class constructor use the default value whenever `explorModeTrigger` is `null`. PiperOrigin-RevId: 363568651 * Remove broken NumericTickProvider.getTicks caching NumericTickProvider.getTicks has vestigal code that attempted to avoid recalculating ticks by checking if the tick parameters have changed. However, its check is always true, and therefore it always recalculates ticks. I attempted to fix the caching, but that broke screenshot tests in charts_web. For now, remove the caching entirely. PiperOrigin-RevId: 363568737 * Expose BasicDateTimeTickFormatterSpec in charts_flutter PiperOrigin-RevId: 364455090 * Internal changes PiperOrigin-RevId: 365704793 * Fix more analysis errors * Fix an implicit cast from `BarRendererDecorator<Object>` to `BarRendererDecorator<D>`. `BarRenderer<D>.barRendererDecorator` should be parameterized on `Object` instead of `D` because it is initialized from a `BarRendererConfig<Object>`. * Fix some code in `SelectionModel.fromConfig` that called `Iterable<E>.contains` with a completely unrelated type, meaning that `contains` would always return false. * Fix some analysis errors in tests. * Add missing `@override` annotations. * Add some missing type parameters to generics. (This is not comprehensive.) * Fix some implicit casts. (This is not comprehensive.) Also, fix a bug that I introduced in my earlier change to `A11yExploreBehavior`'s constructor. When I moved the default value for `exploreModeTrigger` from the parameter list to the initializer list, the constructor body still referenced the argument (which now had a default value of `null`) instead of the initialized member. (I've filed dart-lang/linter#2552 to lint such cases in the future.) PiperOrigin-RevId: 367157475 * Make better use of string interpolation * Replace string concatenation with `+` with string interpolation in some cases I noticed. * Replace `${variable.toString()}` in interpolated strings with just `$variable`. * Remove some unnecessary braces. These change will reduce to the need to use the null-assertion operator (`!`) in an upcoming change to enable null-safety. PiperOrigin-RevId: 367157594 * Make more named parameters required (and make some not required) Annotate named parameters with `@required` in cases where they're actually required (i.e., the callee unconditionally dereferences the argument) and in some cases where all callers already supply arguments. This will avoid needing to make some members nullable when null-safety is enabled. I also removed `@required` from `DateTimeTickFormatter.format`'s `stepSize` parameter since it's not required by the base `TickFormatter.format` method and is therefore unenforceable. Additionally, `LayoutViewConfig`'s constructor used `@required` for its arguments, but almost none of its derived classes supply those arguments. (That these don't generate analysis warnings is a known deficiency of the analyzer when derived classes rely on implicit calls to `super()`.) Even though `LayoutViewConfig` was intended to have required parameters, I instead made them optional to preserve existing behavior and because I did not know what values to pass for each derived class. Finally, initialize some data in tests and add some mock responses that they will no longer return null. This is in preparation for making some types non-nullable. PiperOrigin-RevId: 367160939 * Prefer initializer lists to constructor bodies Initialize class members in initializer lists instead of in constructor bodies when possible: * Initializer lists avoid accidental name shadowing bugs where `member` is use instead of `this.member`. * Initializer lists allow more members to be declared as `final`. In turn, this will avoid needing to declare those members as nullable or as `late` when null-safety is enabled. PiperOrigin-RevId: 367161613 * Add and use construction parameters instead of mutating members Replace code of the form: ```dart var foo = Foo() ..property1 = value1 ..property2 = value2 ..property3 = value3; ``` with ```dart var foo = Foo( property1: value1, property2: value2, property3: value3, ); ``` so that the members can be `final` when possible. This also will allow members to be non-nullable when we enable null-safety. PiperOrigin-RevId: 367161773 * Add a NullablePoint class and use it instead of Point<double> charts_common expects to be able use null coordinate values, but when null-safety is enabled, this will no longer possible with `dart:math`'s `Point` class. Add a `NullablePoint` class to use as a substitute. This might not be the ideal long-term solution, but it should preserve existing behavior for now. I also have replaced only the sites that clearly depended on null coordinates; it is quite possible that there are sites not covered by automated tests that still need to switch from `Point<double>` to `NullablePoint`. PiperOrigin-RevId: 367165943 * Make better use of null-aware operators PiperOrigin-RevId: 367539685 * Explicitly handle null values in more places * Add explicit handling of `null` in cases where there is an obvious way to fail gracefully. * Add some default values/arguments. * Make some functions avoid returning `null`. In some cases I just added `assert`s. Some of these changes aren't strictly necessary since the code apparently works already. However, when null-safety is enabled and types are made explicitly nullable, I think that this is preferable over greater usage of null-assertion operators (`!`). PiperOrigin-RevId: 367542367 * Miscellaneous, non-trivial changes * Replace some `Iterable.forEach` loops with `for-in` loops: * `return`s in `forEach` can be misleading, and `for-in` with `continue` better expresses intent. * Closures cannot use promoted types from the parent environment. When null-safety is enabled, using `forEach` therefore would require unnecessary use of the the null-assertion operator (`!`). * Adjust some comments. * Simplify some `sort` callbacks to use `Comparable.compareTo` instead of performing manual comparisons. * Use redirecting constructors in a couple of places. * Simplify `Timer` cancellation. * Remove an unused parameter. * Other miscellaneous cleanup. PiperOrigin-RevId: 367542429 * Disable null-safety for tests Explicitly disable null-safety for files in `test/` in preparation for enabling it for `lib/`. PiperOrigin-RevId: 367542475 * Mechanical null migration changes Add `?` to many types, add null assertion operators (`!`) in a lot of places, add `late` keywords, and change `@required` to `required` everywhere. To reduce the number of sites that need null assertions, I shadowed member variables with identically named local variables so that a single null assertion will cause type promotion. That is, intead of: ```dart f(member!.x + member!.y + member!.z); ``` When possible, I preferred: ```dart final member = this.member!; f(member.x + member.y + member.z); ``` PiperOrigin-RevId: 367542574 * Roll back null safety migration change Enabling null safety broke some internal tests. PiperOrigin-RevId: 367584842 * Reapply null safety migration change PiperOrigin-RevId: 368179137 * Post-null-migration cleanup * Dart 2.12 allows type promotion to occur from `as` casts, so take advantage of that. This will fix the `unnecessary_cast` violations. * I also noticed that `OrdinalScale`'s constructor has an optional `tickDrawStrategy` parameter but does nothing with it. Fix the constructor to use it to initialize its `tickDrawStrategy` member. * Change another `.forEach` loop to `for-in` so that it can stop iterating early when possible. * It's logically impossible for `RangeAnnotation<D>._annotationMap` to store null values, so checking `_annotationMap.containsKey(key)` is equivalent to checking if `_annotationMap[key]` is non-null, and the latter is slightly simpler. PiperOrigin-RevId: 368324239 * Internal changes PiperOrigin-RevId: 368444143 * Fix build warnings in charts_common PiperOrigin-RevId: 370588847 * Automated g4 rollback of changelist 370588847. WANT_LGTM=any *** Reason for rollback *** Breaks multiple Greentea FE widgets, causing presubmit TAP failures. *** Original change description *** Fix build warnings in charts_common *** PiperOrigin-RevId: 370712865 * Automated g4 rollback of changelist 370712865. *** Reason for rollback *** Rollforward after fix in greentea submitted in cl/370722339. Global presubmit is showing all targets ok after deflakes *** Original change description *** Automated g4 rollback of changelist 370588847. WANT_LGTM=any *** Reason for rollback *** Breaks multiple Greentea FE widgets, causing presubmit TAP failures. *** Original change description *** Fix build warnings in charts_common *** *** PiperOrigin-RevId: 371141851 * Internal changes PiperOrigin-RevId: 371425261 * Internal changes PiperOrigin-RevId: 371617443 * Internal changes PiperOrigin-RevId: 371908750 * Internal changes PiperOrigin-RevId: 372183756 * Internal changes PiperOrigin-RevId: 372426281 * Internal changes PiperOrigin-RevId: 374314810 * Internal changes PiperOrigin-RevId: 375978148 * Internal changes PiperOrigin-RevId: 375997369 * Internal changes PiperOrigin-RevId: 378428877 * Internal changes PiperOrigin-RevId: 378497037 * Migrate deprecated TextTheme API from body1 to bodyText2 This deprecated API is scheduled to be removed in flutter/flutter#83924 PiperOrigin-RevId: 378523673 * Internal changes PiperOrigin-RevId: 379206830 * Internal changes PiperOrigin-RevId: 379784071 * Internal changes PiperOrigin-RevId: 379986913 * Fixed nullability issues in chart classes. PiperOrigin-RevId: 380237923 * Migrate //third_party/dart/charts_flutter to null safety. PiperOrigin-RevId: 381309874 * Migrate //third_party/dart/charts_flutter/examples to null safety. PiperOrigin-RevId: 381509039 * Bump sdk version for nullability support to hopefully get travis tests to pass PiperOrigin-RevId: 381579690 * Fix mockito testing for null safety PiperOrigin-RevId: 382373361 * Fixing tests removing some mockito use PiperOrigin-RevId: 382380502 Co-authored-by: jamesdlin <jamesdlin@google.com> Co-authored-by: Googler <noreply@google.com> Co-authored-by: nolankelly <nolankelly@google.com>
The other day I modified some non-null-safe code from:
to:
It turns out that this was wrong because that "Other work" portion in the constructor body referenced
x
, and because the localx
parameter understandably shadows thex
member, it therefore was not using the expected default value. Oops. I didn't realize this mistake until I later attempted to migrate the code for null-safety.This is probably less of a problem with null-safety enabled, although it could still happen. For example:
Or it could just be something like:
It would be nice if there were some lint that suggested using
this.x
instead ofx
in the constructor body for these situations. In the specific case of constructors, if a parameter name and a member name are the same, it seems rare to me that someone would prefer the parameter; I think it would almost always be a mistake. This isn't fully baked, but I think criteria would be something like:T
has a member variablex
and a constructor with a parameter namedx
. That constructor has an initializer list that initializesx
that is notx = x
orthis.x = x
. (Intention is to lint only if the member and parameter potentially have different values.)T
has a setter namedx
and a constructor with a parameter namedx
. That constructor has a body that assigns tox
.The text was updated successfully, but these errors were encountered: