Dart-code: TODOs not showing up in Problems view

[Skylled]

Expected: TODOs are found by the Dart Analyzer, reported to DC, given a highlight, and listed in the Problems view.

Actual: TODOs are found by the Analyzer, reported to DC, and given a highlight.

.

I'm not sure when this changed, but I've tested on both 2.11 and 2.12 beta 2.
Code version: 1.22.2 (Stable)

[DanTup]

Ok, this is because of #417. Code has changed Hint to now be hidden from the diagnostics list and only used to trigger "suggestions" (which are grey dotted lines in the editor).

We can change this back to Information but then it will get mixed up with the others.

I've opened https://github.com/Microsoft/vscode/issues/48376 (please 👍 it!) but in the meantime I'm going to change these back to Information so they appear (they'll get mixed up in the lints, but if you prefer the current behaviour - them not appearing - you can just turn the setting off).

[Skylled]

Thanks for the quick answer. Thumbed it up.

In the meantime, it would be good to revert the changes of #417, regaining TO-DO visibility at the cost of organization. Also open to other ideas.

Edit: Just noticed you already reverted. Fantastic!

[DanTup]

Yup; planning on an updated v2.12 beta probably tomorrow or Wed with latest changes; release may be next week now.

[DanTup]

Response from MS:

Most use warning for lint messages. Why isn't that an option for you?

Any opinions on that?

[Skylled]

I'm sure others will have input on the subject, but personally as long as TODOs are below all other hints/lints/warnings/errors, some mixed layering on the others should be fine. (Of course, native To-do support from VSC would be best but that's a different issue.)

[DanTup]

Looking at this again, the descriptions in VS Code are this:

Error - Something not allowed by the rules of a language or other means.
Warning - Something suspicious but allowed.
Information - Something to inform about but not a problem.

So I think putting lints in Information is kinda wrong anyway - the user has decided these things should be problems, so they should be warnings.

So, I'm thinking of ignoring severity from the server and doing this:

"CHECKED_MODE_COMPILE_TIME_ERROR" = Error
| "COMPILE_TIME_ERROR" = Error
| "HINT" = Warning
| "LINT" = Warning
| "STATIC_TYPE_WARNING" = Warning
| "STATIC_WARNING" = Warning
| "SYNTACTIC_ERROR" = Error
| "TODO" = Information

@bwilkerson Do the above categorisations make sense to you? From previous comments, I think you said hints are things that are basically warnings but weren't in the Dart spec as such (like unreachable code)?

[bwilkerson]

Those make sense, with one caveat: users can change the severity of errors, warnings, hints and lints in the analysis_options.yaml file, and this approach would discard those changes. I don't know whether any users actually use that ability, though, so the effect might be unnoticed.

[DanTup]

users can change the severity

Aha! That's a good point - I saw that in a repro someone sent me recently. In that case, I wonder if we're better leaving as things are and let users promote lints to warnings in the analysis options if they want?

I presume in IntelliJ the lints won't be appearing as warnings?

[bwilkerson]

I think they show up with the same visual appearance as hints, but I'd have to double check.

I looked over the issue again to try to answer your question of whether we should leave things as they are. Given those definitions, Dart's "warnings" ought to be displayed as errors. (The spec's distinction between "error" and "warning" is, IMO, not one that most users care about.) I'm hoping that we can revisit this on the Dart side after 2.0.

[DanTup]

Makes sense, but... if users can modify how things appear and we start mapping warnings to errors, won't that become really confusing if they're trying to modify them? Is there any good info on what can be re-mapped and how? The page I found seems kinda vague; tells me how to ignore TODOs but not much else (though that also makes me wonder whether we should even have dart.showTodos in Dart Code or let them use that!

[bwilkerson]

Is there any good info on what can be re-mapped and how?

Not that I know of. We have not done a very good job of documenting the analysis options file.

The page I found seems kinda vague ...

I suspect that we were intentionally vague. The feature was originally added to allow users to ignore hints that they didn't want to see. But, in the age old tradition of software development, the feature was designed to be very general. It isn't a good idea for users to ignore or downgrade errors, but the feature supported it. Rather than fix the feature, we downplayed the fact that it was possible.

... though that also makes me wonder whether we should even have dart.showTodos in Dart Code or let them use that!

One of the trade-offs here is granularity. If todos are ignored in the analysis options file, then they are ignored for everyone analyzing code in that package. If there is a per-user option in the IDE, then it doesn't impact other users. Whether users need that level of control is a whole different question.

[DanTup]

Ok, I think for now maybe I'll leave as-is. Seems like whatever I do it'll be wrong for some, and at least those it's wrong for today have already dealt with it, I don't want to upset a bunch of new people.

@Skylled Based on the above I think you may be able to make lints appear as warnings with the analysis_options file, so hopefully that'll somewhat addresses your issue?

If todos are ignored in the analysis options file, then they are ignored for everyone analyzing code in that package.

Good point :)

[Skylled]

Okay so the gist of it is that TODOs will come back in their pre-417 state as Information, and that I can use analysis_options.yaml to tune my lints to be Warnings?

Something like:

analyzer:
  strong-mode: true
  language:
    enableSuperMixins: true
linter:
  # Full list available at http://dart-lang.github.io/linter/lints/options/options.html.
  rules:
    - always_declare_return_types: warning
    - always_specify_types: warning

[bwilkerson]

The idea is correct, but the syntax is off. Unfortunately, it looks like:

analyzer:
  strong-mode: true
  language:
    enableSuperMixins: true
  errors:
    always_declare_return_types: warning
    always_specify_types: warning
linter:
  # Full list available at http://dart-lang.github.io/linter/lints/options/options.html.
  rules:
    - always_declare_return_types
    - always_specify_types

It would be much nicer if we had support for customizing the lints in the list of enabled lints, but we don't currently have that. So, unfortunately, you end up having to duplicate the list of rules.

[Skylled]

Thanks for the clarification, @bwilkerson!

[DanTup]

@Skylled Let me know if this works ok. It's not perfect, but maybe we can revisit in future (it seems like the best ways to improve it will involve the analyzer and not just Dart Code).

[entonio]

Hi @DanTup. I've been having this problem for a while now. It happened only in one of my machines, the other was OK, but now they're both the same.

• The analyzer finds the TODOs and assigns to them whatever severity I specify in my yaml;
• But the type is TODO;
• For some reason, the errors with type *TODO *are not counted in the analyzer's final count, but that's just logging afaict;
• vscode doesn't show the TODOs, though it will show other errors if present.

I'm running the latest versions of everything. I believe my machine that didn't have this issue was running an outdated version of something, but now it's not. I can't tell what.

[entonio]

PS - your related issue got autoclosed, I think.

[DanTup]

@entonio could you file a new issue with details of exactly what you're seeing and a code/analysis_options sample? Please also check the dart.showTodos in VS Code is set to true. Thanks!