Un-useful static warning for final fields?

[Bob Nystrom]

The language specifies that any final field must have an explicit initializer in the constructor. So this generates a warning:

class Foo {

  final String name;

  Foo();

}

[warning] The final variable 'name' must be initialized (foo.dart, line 3, col 3)

Because, presumably, you forgot to initialize name and this class will never do anything useful.

But now consider this:

class Foo {

  final String name;

  Foo(this.name);

  Foo.nameless();

}

I still get a warning here. But now this code is deliberate, useful, and minimal. Adding an explicit name = null initializer to Foo.nameless() accomplishes nothing.

This came up in a real code review with a class that had two constructors. One was for an "undefined" state for the class where all of its fields were left null. To get the static warnings to shut up, the author had to add a painfully long list of " = null" initializers.

Does this warning actually prevent a common user error? Is it helpful in the non-false positive cases? If not, how about we just remove it?

If there is agreement that this isn't useful, I can write a little DEP for it. It would mean just removing this from the spec (section 10.6):

Each final instance variable f declared in the immediately enclosing class must have an initializer in k’s initializer list unless it has already been initialized by one of the following means: 

• Initialization at the declaration of f.

• Initialization by means of an initializing formal of k.

or a static warning occurs.

Thoughts?

– bob

[Brian Slesinsky]

I believe I rely on the current behavior while adding a new final field, or at least I did in Java:

- Add the new field.

- Wait for the IDE to show the warnings.

- While warnings exist:

  - Decide what should happen for that constructor.

  - Fix it so the warning goes away.

Deliberately introduced errors seem pretty helpful as a to-do list (a habit I picked up while working in Java). I think it's important to consider what should happen for each final field and generative constructor. If there are lots of fields and several constructors, it seems pretty easy to miss a case. But if I didn't have them, I'm sure I'd adjust.

(Using Atom, it's still a bit awkward since I need to reanalyze sources occasionally to be sure all the warnings are gone. If that could be fixed it would be a lot smoother.)

- Brian

--
You received this message because you are subscribed to the Google Groups "Dart Core Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to core-dev+u...@dartlang.org.

[Günter Zöchbauer]

In my experience it's very rare that I want final fields initialize with null. Every time I got this error I was glad, because it indicated a bug. 

[Rasmus Eneman]

Maybe it could be changed to be happy if any of the constructors would initialize that variable rather than all?

Would catch the error while still allowing the use case Bob shows.

--

You received this message because you are subscribed to the Google Groups "Dart Core Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to core-dev+u...@dartlang.org.

--

Rasmus Eneman

[Günter Zöchbauer]

I don't want it to be silenced just because at least one of the constructors is not buggy ;-)  

[Filipe Morgado]

I find the warning useful. I prefer explicit assignments.

[Bob Nystrom]

OK, sounds like everyone is happy with the status quo. Thanks!

– bob

On Sat, Sep 26, 2015 at 11:15 AM, Filipe Morgado <pix...@gmail.com> wrote:

I find the warning useful. I prefer explicit assignments.

[Patrice Chalin]

I faced this issue when implementing DEP 30 (non-null) in the analyzer. For sake of consistency I decided to keep the requirement that all final fields be explicitly initialized (B.3.4(a)). But if/once DEP 30 is introduced it may be worth while revisiting this initialization rule. We may be able to have our cake and eat it too so that for

class Foo {

  final String name;        // warning (non-null type)

  final String? nickName;   // no warning (nullable type)

  Foo(this.name);

  Foo.nameless();

}

would generate a warning for name's lack of initialization (via nameless()) but not nickName.

-- Patrice

[Bob Nystrom]

On Mon, Sep 28, 2015 at 1:18 PM, Patrice Chalin <pch...@gmail.com> wrote:

But if/once DEP 30 is introduced it may be worth while revisiting this initialization rule. We may be able to have our cake and eat it too so that for

class Foo {

  final String name;        // warning (non-null type)

  final String? nickName;   // no warning (nullable type)

  Foo(this.name);

  Foo.nameless();

}

would generate a warning for name's lack of initialization (via nameless()) but not nickName.

+1. That's a great point. I think if we have non-nullable types, it makes sense to recast this warning as a type error, not a failure-to-initialize error.

Or, another way to look at it, is that the error we have today basically is a non-nullable type error, we just don't have non-nullable types, so we use "initialized at all" as a heuristic to approximate it.

Cheers!

– bob

[Filipe Morgado]

+1