prefer_const_constructors false positive

[apwilson]

The following code triggers the prefer_const_constructors linter rule on the initial assignment of foo:

///
class Foo {
  ///
  const Foo();
}

///
class Bar {
  ///
  Foo foo = Foo();    // Triggers prefer_const_constructors

  ///
  set changeFoo(Foo newFoo) {
    foo = newFoo;
  }
}

As I thought new/const was optional and inferred, this appears to be a false positive.

[matanlurey]

Yes, new is optional, but const is only optional in a const context. Consider:

main() {
  var foo = /* new */ Foo();
  var foos = const [
    /* const */ Foo(),
  ];
}

Sorry about the confusion. I think @leafpetersen is working on an update to this feature for the language site/etc.

[apwilson]

So I have a related false positive then:

///
class Foo {}

///
class Bar {
  ///
  final Foo foo;

  ///
  const Bar({this.foo});
}

///
class BarFactory {
  ///
  Bar makeBar() {
    return Bar(foo: Foo()); // false positive saying I should make Bar const.  But if I make Bar const, Foo will try to become const and it can't
  }
}

[a14n]

@apwilson I don't see the lint with your last example. What's the version of your Dart SDK and the linter package if you have one specified?

[apwilson]

Our dart version is: 46ab040

[apwilson]

linter version is 0.1.49 I think?

[a14n]

I can reproduce it on master of linter. It's related to ConstantVerifier that does not trigger error for const Bar(foo: Foo()). @bwilkerson could you take a look?

[bwilkerson]

I have identifier the cause of the problem. The expression cannot be const because Foo can't be const, but the error indicating the problem is not produced by the ConstantVerifier. Hence, it does not appear in the list of "constant related errors" and the lint rule cannot tell that the expression cannot be const.

Fixing this problem is a bit bigger than I have time for at the moment.

[a14n]

@bwilkerson thanks for your explanation.

[a14n]

As ConstantVerifier is also used by prefer_const_literals_to_create_immutables it may also explain prefer_const_literals_to_create_immutables false positives mentioned with optional new/const in #57712

[a14n]

I tried to fix it with this simple diff:

diff --git a/pkg/analyzer/lib/src/dart/constant/evaluation.dart b/pkg/analyzer/lib/src/dart/constant/evaluation.dart
index b65aa46..eb2e46b 100644
--- a/pkg/analyzer/lib/src/dart/constant/evaluation.dart
+++ b/pkg/analyzer/lib/src/dart/constant/evaluation.dart
@@ -417,6 +417,11 @@ class ConstantEvaluationEngine {
       ConstantVisitor constantVisitor,
       ErrorReporter errorReporter,
       {ConstructorInvocation invocation}) {
+    if (!constructor.isConst) {
+      errorReporter.reportErrorForNode(
+          CompileTimeErrorCode.CONST_WITH_NON_CONST, node);
+      return null;
+    }
     if (!getConstructorImpl(constructor).isCycleFree) {
       // It's not safe to evaluate this constructor, so bail out.
       // TODO(paulberry): ensure that a reasonable error message is produced

But this change triggers errors in several tests that need more knowledge and time than I have to fix them.

Any help would be very welcome :)

[dgrove]

Has the analyzer fix unblocked this?

[a14n]

The fix unblocked but some other changes need to be done (see dart-archive/linter#1035)

[pq]

Thanks @a14n . Right, this is gated by an analyzer release; @bwilkerson and @devoncarew and I need to get together on how to make one happen.

[pq]

analyzer published and our DEP bumped; are we good here now @a14n ?

[a14n]

Yes!

[pq]

Super. Can I close?

[a14n]

I think so.

[pq]

Good deal. Thanks for your perseverance and @apwilson for your continued patience!