Change errors to warnings

[eernstg]

[Edit eernstg Oct 11 2018] The currently valid list of situations which were specified as errors and should now be warnings is the following:

import_duplicated_library_named
mismatched_getter_and_setter_types
mismatched_getter_and_setter_types_from_supertype
missing_enum_constant_in_switch
non_void_return_for_operator
non_void_return_for_setter
return_without_value

Below is the pre-edit text of this issue.

---

The following identifiers (present in the implementation of the analyzer) indicate 13 situations where Dart 2 still has warnings (contrary to commit a07b2a1 where all warnings were turned into errors, as had been the plan for a long time):

conflicting_dart_import
final_not_initialized
final_not_initialized_constructor (note that this prefix matches 3 codes)
import_duplicated_library_named
mismatched_getter_and_setter_types
mismatched_getter_and_setter_types_from_supertype
missing_enum_constant_in_switch
non_void_return_for_operator
non_void_return_for_setter
return_without_value
mixin_return_types

The language specification should be updated to make them warnings again, and the dynamic semantics specified (or we should confirm that the existing rules already cover these situations).

Alternatively, we could delete these errors from the language specification, and leave it to the linter to flag them, if desired. For that we'd obviously still need to ensure that the dynamic semantics is well-defined.

[eernstg]

See #34319: We also need to update invalid_returns.md!

[askeksa-google]

The conclusion to #34403 seems to be that type arguments after the constructor name in a named constructor invocation should also be downgraded to a warning. Should this go on the list as well? From the point of view of the spec, I guess it could just not say anything about it (so it falls outside of the syntax), but we let our tools accept it with a warning. Another possibility could be to describe it but say that it is not supported (yet).

[eernstg]

I wonder if this

.. type arguments after the constructor name in a named constructor
invocation should also be downgraded to a warning ..

would push generic named constructors into the remote future? I think that would be unfortunate, and I hope that we would at least be able to introduce generic named constructors and then only warn when a given actual type argument list could only be intended for the class, because the constructor isn't generic (and the class is generic).

@lrhn, @leafpetersen, do you agree that we should aim to introduce generic named constructors? We may or may not be able to do exactly what I described, but I'd like to have some indication that we're not making generic named constructors a no-no without noticing it.

[leafpetersen]

I think we can fix the incorrect type argument issue, we just need to roll it out:

• keep it as a warning for a few dev cycles
• fix all known bad code
• switch it to an error

[askeksa-google]

A list of breaking bug-fixes has been posted in #34611. The language team should consider whether any of these errors should go on this list.

[eernstg]

This CL adjusts dartLangSpec.tex such that the compile-time errors mentioned above are changed to warnings.

The following are not covered by this CL (I don't know which error they would refer to): conflicting_dart_import and mixin_return_types.

[kmillikin]

Is the Dart 2.1 decision then that final_not_initialized and final_not_initialized_constructor are errors after all? Or warnings?

[askeksa-google]

More precisely: In which of the following situations should it be an error not to have an initializer (or appropriate constructor initialization) for a final variable:

• Local variable?
• Top-level variable?
• Class variable?
• Instance variable?

[askeksa-google]

Some more context:

All four situations were warnings in the analyzer in 2.0 and have been errors since 1b8f302 (August 29).

The first three were errors in the CFE in 2.0. The last one was not detected by the CFE in 2.0, but it has been an error since f8a96b8 and eb37da8 (August 17).

So if we define a breaking fix as something that was not detected by the analyzer in 2.0 (which is the definition used in #34611), it will not be breaking to make them all errors.

[lrhn]

I'd say all of them. If the analyzer already makes it an error, and has for this long, I'd just follow suit.

It should be an error to not initialize any final static (clas or library) or local variable.
It should be an error if a class has a final instance variable with no initializer and a generative constructor which does not initialize that variable (one error per constructor, and it includes the default constructor if the class declares no constructors).

I guess that means a class with a non-initialized final instance variable declaration and no generative constructor (due to having only factory constructors) will perhaps not get an error. If it's used to derive a mixin, then the mixin application will have a constructor which fails to initialize the field. Otherwise the class and instance variable are useless, since the class can neither be instantiated nor extended.

[askeksa-google]

Good. This is exactly what is implemented in the analyzer and CFE, with the slight variation that if none of the generative constructors initialize the instance variable, only one error is reported, at the variable.

[eernstg]

FYI: The ongoing specification CL has been updated to make the situations concerned with uninitialized final variables errors again.

[eernstg]

Said CL was landed as e4e82da.