`avoid_redundant_argument_values` false positive for code that is not yet nnbd but uses an nnbd library

[Hixie]

STEPS TO REPRODUCE

• git clone https://github.com/flutter/flutter.git
• git checkout db426693c5901076b907b6b8de8d482522ffff1c
• cd flutter/packages/flutter_tools
• Add avoid_redundant_argument_values: true to the analysis_options.yaml file.
• Run dart fix --apply
• Run flutter analyze

EXPECTED RESULTS

No analyzer warnings.

ACTUAL RESULTS

You get several analyzer warnings. Some are because of https://github.com/dart-lang/sdk/issues/47398, some are interesting warnings like unused imports are optional parameters never being given, that kind of thing, which you would expect as a result of the fix.

However, a whole bunch of them are missing_required_param warnings, where null-safety-aware code with arguments of the form required Foo bar which was being called from non-migrated code, with the argument set to null, now is causing a complaint because the argument was removed from the call. For example, test/integration.shard/plist_parser_test.dart has this change applied:

index 1445c7eee3..a5ccde362a 100644
--- a/packages/flutter_tools/test/integration.shard/plist_parser_test.dart
+++ b/packages/flutter_tools/test/integration.shard/plist_parser_test.dart
@@ -39,21 +39,20 @@ void main() {
   // works with the way we're calling it.
   File file;
   PlistParser parser;
   BufferLogger logger;

   setUp(() {
     logger = BufferLogger(
       outputPreferences: OutputPreferences.test(),
       terminal: AnsiTerminal(
         platform: const LocalPlatform(),
-        stdio: null,
       ),
     );  
     parser = PlistParser(
       fileSystem: fileSystem,
       processManager: processManager,
       logger: logger,
     );
     file = fileSystem.file('foo.plist')..createSync();
   });

...even though AnsiTerminal is defined as:

class AnsiTerminal implements Terminal {
  AnsiTerminal({
    required io.Stdio stdio,
    required Platform platform,
    DateTime? now, // Time used to determine preferredStyle. Defaults to 0001-01-01 00:00.                                                                                                               
  })
    : _stdio = stdio,
      _platform = platform,
      _now = now ?? DateTime(1);

//...

I tried to make a simple test case but none of my attempts succeeded; in controlled conditions, I was able to get the analyzer to give me the (bogus) avoid_redundant_argument_values lint but never able to get dart fix to act on it.

[asashour]

Regarding the null argument issue:

// a.dart
class A {
  int f;
  A({required this.f}) {
    print('called');
    // print(f.isEven); // this gives runtime error in either way!
  }
}

// @dart=2.9
import 'a.dart';

void f() {
  A(); // <-- should we have 'f: null'?
}

void main() {
  f();
}

There are few things:

1. Whether the removal of required argument is correct, or the fix to add it is incorrect. Since the sample is executed without errors, that means is it redundant. So the removal is correct, but the suggestion to add it is incorrect.

2. AddMissingRequiredArgument is not configured for bulk, I guess this is because what the default value to add would be, so it is left to the user.

3. The parameter is not nullable, and using it will trigger a runtime error. However the caller is not NNBD (use at your own risk). I guess nothing to be done here.

Suggestion:

• AddMissingRequiredArgument should not be triggered in this case (when the caller is not NNBD)

[scheglov]

@bwilkerson

[Hixie]

I think the language conceptually implies required parameters have no default value, even though at some level they sort of do. I would lean towards saying that if the parameter is marked required, the linter should never consider callers to have a matching redundant argument, even if the specified value does technically match the "actual" default value.

[bwilkerson]

I agree. This sounds like an issue with the lint rule. @pq If you agree then we should move this issue to the linter's issue tracker.

[pq]

This sounds like an issue with the lint rule.

Agreed.