Skip to content
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

feat: implement stats per linter with a flag #4341

Merged
merged 5 commits into from Feb 2, 2024

Conversation

mostafa
Copy link
Contributor

@mostafa mostafa commented Feb 1, 2024

In this PR I added a flag that enables basic stats collection and printing per linter using the --show-stats-per-linter flag to the run subcommand. This only prints the stats to the terminal after running all the linters.

Note to the reviewers:
I tried implementing it using e.runAnalysis to return the stats from the run.Run, yet it made things more complicated. I can still do it if you prefer that approach and I'd be happy to have your feedback. Also, I can either update the printers.Print interface to accept the stats or add a extra field (somewhere), so that the stats are printed based on the formatter used, and not just to the terminal.

Closes #2924.

Copy link

boring-cyborg bot commented Feb 1, 2024

Hey, thank you for opening your first Pull Request !

@CLAassistant
Copy link

CLAassistant commented Feb 1, 2024

CLA assistant check
All committers have signed the CLA.

@ldez ldez self-requested a review February 1, 2024 15:32
@ldez ldez added area: CLI Related to CLI enhancement New feature or improvement labels Feb 1, 2024
@ldez ldez changed the title Implement stats per linter with a flag cli: implement stats per linter with a flag Feb 2, 2024
@ldez ldez changed the title cli: implement stats per linter with a flag feat: implement stats per linter with a flag Feb 2, 2024
@ldez
Copy link
Member

ldez commented Feb 2, 2024

Hello,

I made some modifications:

  • Replaced --show-stats-per-linter by --show-stats.

  • Changed the output:

    Before
    Stats per linter:
      goconst: 1
      tagalign: 1
      inamedparam: 5
      musttag: 1
      perfsprint: 50
      nolintlint: 1
      typecheck: 2
      gofumpt: 1
      protogetter: 20
      testifylint: 50
    

    Note: the order of linters is not deterministic (because of a map).

    Stats per linter:
      no issues
    
    After
    132 issues:
    * goconst: 1
    * gofumpt: 1
    * inamedparam: 5
    * musttag: 1
    * nolintlint: 1
    * perfsprint: 50
    * protogetter: 20
    * tagalign: 1
    * testifylint: 50
    * typecheck: 2
    

    Note: the order of linters is deterministic (alphabetic order).

    0 issues.
    
  • Added the option to the documentation.

@mostafa
Copy link
Contributor Author

mostafa commented Feb 2, 2024

@ldez

Awesome! Thanks! 🙏

Copy link
Member

@ldez ldez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution to golangci-lint.
Your PR will benefit many users.

@mostafa
Copy link
Contributor Author

mostafa commented Feb 2, 2024

@ldez

Thanks for tending to it so quickly! 🙏

pkg/commands/run.go Outdated Show resolved Hide resolved
Co-authored-by: Anton Telyshev <anton.telishev@yandex.ru>
@ldez ldez enabled auto-merge (squash) February 2, 2024 19:23
@ldez ldez merged commit b3ffe70 into golangci:master Feb 2, 2024
12 checks passed
@@ -489,6 +494,31 @@ func (e *Executor) createPrinter(format string, w io.Writer) (printers.Printer,
return p, nil
}

func (e *Executor) printStats(issues []result.Issue) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to add a unit or integration test?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need for now, we will wait for users' feedback before doing something because the current implementation is simple and it can evolve with feedback.

@mostafa mostafa deleted the show-stats-per-linter branch February 2, 2024 23:16
Antonboom pushed a commit to Antonboom/golangci-lint that referenced this pull request Mar 3, 2024
Co-authored-by: Fernandez Ludovic <ldez@users.noreply.github.com>
@ldez ldez modified the milestone: v1.56 Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: CLI Related to CLI enhancement New feature or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Report count of issues shown for a particular linter type
5 participants