Analyzer: prefer_final_fields confused when used 'part'

[siqwin]

The analyzer signals: Private field could be final (prefer_final_fields) when using part directive. Example:

test.dart

part 'test.g.dart';

class A {
  int _id;
  A(this._id);
}

test.g.dart

part of 'test.dart';

class B {
  void update(A instanceA) {
    instanceA._id = 1;
  }
}

• Dart VM version: 2.7.0 (Mon Dec 2 20:10:59 2019 +0100) on "windows_x64"

[bwilkerson]

@pq @scheglov

Interesting case. The linter doesn't really support parts well because it looks at each compilation unit in isolation rather than looking at all of the parts together. This doesn't appear to be a problem in most cases, but is a problem when the rule needs to find uses of private variables. (I don't know whether there are other rules that have this characteristic.)

Short term we might want to disable the rule in compilation units that have parts.

Short of re-architecting the linter, we might need to do something like adding an API (to LinterContext?) to allow the rule to find references to private elements in other parts. Other ideas?

[pq]

Short term we might want to disable the rule in compilation units that have parts.

That seems like a reasonable measure.

Not idea but I don't really see a re-architecting happening soon (or necessarily being the right approach). I'd be curious what @scheglov thinks though...

[scheglov]

The right way to support such multi-part libraries and non-local lints is to support multi-pass lints. The first pass gathers information across all parts, and the second pass reports lints.

Information can be gathered in the LintRule itself, rules are never used concurrently, and just like we set ErrorReporter reporter before we start using a LintRule, we can initialize, fill, use, reset this additional information.

We would need an additional interface / API for such rules, just like we added NodeLintRule for sharing AST visitor and subscriptions.

We planned to do something like this, with LinterContext.allUnits, but the only use of it in avoid_private_typedef_functions does not look like nice to me. For every private FunctionTypeAlias or GenericTypeAlias we visit all parts and count how many times it is used in the whole library. This works, but is more expensive than it could be. Although private typedefs are probably rare, so it might be fine. But private fields are much more common.