Skip to content

Timings script should highlight slow lint rules #57765

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
srawlins opened this issue Aug 6, 2018 · 5 comments
Open

Timings script should highlight slow lint rules #57765

srawlins opened this issue Aug 6, 2018 · 5 comments
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-linter Issues with the analyzer's support for the linter package P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug

Comments

@srawlins
Copy link
Member

srawlins commented Aug 6, 2018

I think we could colorize the expanded output (behind a flag?):

/Users/srawlins/code/dart-linter/foo/lib/src/foo/dir/x.dart 1:8 [lint] Document all public members.
String addddddd() => 'yay';
       ^^^^^^^^

1 file analyzed, 1 issue found, in 6472 ms.

Maybe colorize the text being indicated red? addddddd? Or make it bold?

And the --stats output, as part of this TODO:

https://github.com/dart-lang/linter/blob/a088573c6628935a634d642d0863b7fd58b106a6/lib/src/formatter.dart#L96

we could also colorize/shame slow linter rules

------------------------------------------------------
Timings                                             ms
------------------------------------------------------
public_member_api_docs                             180
invariant_booleans                                   7
directives_ordering                                  4
non_constant_identifier_names                        2
file_names                                           1
test_types_in_equals                                 0
------------------------------------------------------
Total                                               32
------------------------------------------------------

Would it be acceptable to add a dependency on ansicolor and start with minimal colors (my two examples)?

@pq
Copy link
Member

pq commented Aug 7, 2018

I love the idea of colorizing output (and am a big fan of ansicolor). One concern is that, in general, we're trying to steer folks away from depending too much on the linter binary itself, preferring the dartanalyzer as the single front-end for linting. (See #57427 for some discussion but feel free to ping me for more context.) It'd be more work obviously, but I wonder if we could consider adding color to the analyzer CLI?

/cc @bwilkerson

@pq pq added the type-enhancement A request for a change that isn't a bug label Aug 7, 2018
@bwilkerson
Copy link
Member

I'm not sure why it would be more work, but yes, the command-line analyzer is the right place for this work.

@srawlins
Copy link
Member Author

srawlins commented Aug 7, 2018

Ah I see, certainly. What brought me here actually, was the TODO on the --strict flag. The TODO doesn't specify an idea on how to shame a slow lint rule. I had the idea to color it red. Are there other ideas?

------------------------------------------------------
Timings                                             ms
------------------------------------------------------
public_member_api_docs                             180!!!
invariant_booleans                                 180 SHAME
directives_ordering                                180 (that's way slow; consider disabling)
------------------------------------------------------
Total                                              720
------------------------------------------------------

@pq
Copy link
Member

pq commented Aug 7, 2018

I'm not sure why it would be more work, but yes, the command-line analyzer is the right place for this work.

OK. I guess you're right. Not more, just different... 😄

Are there other ideas?

I'm not sure. At the moment, I think very few folks look at the benchmarking bot so coloring may not be quite enough anyway. I guess it comes down to what identifying slow lints would do for us. Ideally I think this information would be most useful if tied back to the lint description so that folks can use timings as additional criteria when choosing a lint rule set. I guess another potential would be if we tracked trends or captured a baseline so that we could flag regressions --- in this case by failing the build. Other ideas?

@srawlins
Copy link
Member Author

srawlins commented Aug 7, 2018

Yeah I think that adding a group "expensive" or "slow" will help the user. Maybe --stats doesn't need anything more then.

@srawlins srawlins changed the title Colorize some output Timings script should highlight slow lint rules Jan 18, 2022
@srawlins srawlins added the P3 A lower priority bug or feature request label Sep 25, 2022
@devoncarew devoncarew added devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. labels Nov 18, 2024
@devoncarew devoncarew transferred this issue from dart-archive/linter Nov 18, 2024
@bwilkerson bwilkerson added area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. and removed legacy-area-analyzer Use area-devexp instead. labels Feb 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-linter Issues with the analyzer's support for the linter package P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

4 participants