Don't allow "ignore:" to suppress errors

[stereotype441]

The "ignore:" syntax was intended to allow users to suppress warnings, hints, and lints; and nearly all cases that is how it is used. However, it currently allows errors to be suppressed as well. This creates the risk that genuine errors will creep into users' code bases due to inadvertent use of "ignore:". We would like to change the behavior of "ignore:" so that it cannot suppress errors.

Note that before we can fix this we must address #26980 (since Flutter is currently using the "ignore:" syntax to work around that issue). To work around this issue, we plan to change analyzer's behavior to match the VM's (so that enums may be equality compared inside constants). The issue will remain open, since the spec either needs to be updated to match this behavior, or we need to change the VM and analyzer to disallow it and find a different way to meet Flutter's needs.

[pq]

So this may be contentious (and I'm sorry it's taken me over a year to discover this) but I'm inclined to say this is working very much by design and provides a pretty valuable escape hatch for folks like Flutter and anyone playing with bleeding edge functionality.

If this is something we wanted to enforce tactically, I'd suggest creating a lint that flags ignored errors...

@stereotype441 : how does that sound?

[zoechi]

I also think this is problematic, especially with code generation which is now widely used
where for example non-existing identifiers are imported which causes errors.
This might look differently when the analyzer supports generated code in .dart-tool/

[matanlurey]

Errors are things that would prevent compilation.

[natebosch]

I think anything that is going to blow up in the CFE should not be ignorable - can we revisit this?

[srawlins]

Still blocked on #26980; flutter ignores CONST_EVAL_THROWS_EXCEPTION.

[pq]

To reiterate, I think the feature is working as intended as is and would push back against changing it. In practice, I don't see the benefit in making the mechanism more complex. Are there any motivating examples?

[matanlurey]

It made sense in Dart 1 where compile-time errors could be ignored (in some cases), or there were bugs in what the analyzer and the runtimes (Dart2JS or the VM) considered an error - i.e. the analyzer was sometimes stricter than the language. I see #26980 is still an example of that.

For example, there is no value in the following:

void main() {
  // ignore: invalid_assignment
  String x = 5;
}

Your code cannot compile. CFE will fail, as well all the tools that use it.

Imagine you could write:

// ignore: invalid_syntax
this + is - not ~ dart * code() [[[]]]

What would be the value in allowing a user to ignore syntax errors?

One problematic aspect is that ignore: xxx can be used ignore false-positive hints/lints/bugs which do not affect the compilers or CFE, but we don't want to train users they can ignore: xxx anything they want (i.e. compile-time errors).

[matanlurey]

(If we decide to close this issue, we would likely ban using // ignore: xxx internally at all)

[matanlurey]

/cc @MichaelRFairhurst who we had a long-ish conversation with about this meta issue.

[MichaelRFairhurst]

Yeah I support this change. The CFE will not honor it. While we're at it, I think we should remove the ability to remap error codes. (Changing warnings and hints and lint's should be fine). I suppose if we wanted to really make this long tail, we should look at whether analysis_options.yaml somehow coordinate with the CFE, so that things like --no-raw-types or crazier things like --invariant-generics would be understood by both and not one or the other, and warnings mapped to errors would be errors in both worlds. On Aug 23, 2018 3:14 PM, "Matan Lurey" <notifications@github.com> wrote: /cc @MichaelRFairhurst <https://github.com/MichaelRFairhurst> who we had a long-ish conversation with about this meta issue. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#27218 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABjWe85SDt_xUWnUu3lBR8huVBHh_p2-ks5uTxs-gaJpZM4Jy4_o> .