pylint-dev / pylint

It's not just a linter that annoys you!
https://pylint.readthedocs.io/en/latest/
GNU General Public License v2.0
5.26k stars 1.12k forks source link

Document how to register options for Reporter plugins (by creating a bare checker) #5829

Open teake opened 2 years ago

teake commented 2 years ago

Question

I'm trying to add a configuration option to my Reporter plugin, but am drawing blanks on how to do this. This seems to be possible for Checker plugins [1], but for Reporter plugins (i.e. those extending BaseReporter) I could not find any documentation. The hook load_configuration is available but that seems to be for overriding existing options [2]. How do I add a new CLI option to a Reporter plugin? Thanks!

[1] https://pylint.pycqa.org/en/latest/how_tos/custom_checkers.html#write-a-checker [2] https://pylint.pycqa.org/en/latest/how_tos/plugins.html#how-to-write-a-pylint-plugin

Documentation for future user

I expected this to be documented at https://pylint.pycqa.org/en/latest/how_tos/plugins.html#how-to-write-a-pylint-plugin.

Additional context

I'm attempting to write a reporter plugin that outputs JSON in a format that SonarQube accepts.

The issue severity needs to be customizable, hence the need for a CLI option.

DanielNoord commented 2 years ago

Hi @teake,

I don't think we currently support this. It might be something we could/should look into. However, we're currently working on migrating our complete configuration parsing/registering from optparse to argparse. This is also likely going to change how options can/should be registered.

We're still in the first stage of this process, but I'll add this issue to the todo list so that we will at least take a decision on this when we are able to.

In the meantime, perhaps you could add a checker plugin that just passes but does register an option? We're also thinking of changing our own --json output (see #5796 and #4741). I'm not sure what format SonarQube needs specifically, but perhaps the necessary stuff can be included in our updated --json output?

teake commented 2 years ago

Hi @DanielNoord, thanks for the prompt reply. It took me a bit longer to reply, but I wanted to finish the plugin first; it's now available on https://github.com/omegacen/pylint-sonarjson.

Adding a checker works perfectly indeed, thanks for the tip!

Most of the info that SonarQube needs is already in the standard JSON output, but the structure and key names are different. You can find the SonarQube specification here: https://docs.sonarqube.org/latest/analysis/generic-issue/.

One thing that is missing from the PyLint output, and the reason I wrote the plugin, is the issue severity. This must be configurable per message ID before importing into SonarQube -- you don't want all the PyLint issues set to MAJOR severity for example. But the severity of the types of issues is not something you can dictate or infer from PyLint itself; it's something the owner of the Quality Profile in SonarQube has to configure. Hence I do not think it makes much sense for PyLint to be able to output SonarQube-compatible JSON solely by itself. You will need user input for this. Without configuring the severities, you might as well use the build-in pylint import functionality of SonarQube (which puts all issue severities to MAJOR).

Btw, the severity of PyLint issues used to be configurable from within SonarQube, but they removed this functionality in recent releases.

DanielNoord commented 2 years ago

Hi @DanielNoord, thanks for the prompt reply. It took me a bit longer to reply, but I wanted to finish the plugin first; it's now available on https://github.com/omegacen/pylint-sonarjson.

Good job on finishing it!

Adding a checker works perfectly indeed, thanks for the tip!

Most of the info that SonarQube needs is already in the standard JSON output, but the structure and key names are different. You can find the SonarQube specification here: https://docs.sonarqube.org/latest/analysis/generic-issue/.

One thing that is missing from the PyLint output, and the reason I wrote the plugin, is the issue severity. This must be configurable per message ID before importing into SonarQube -- you don't want all the PyLint issues set to MAJOR severity for example. But the severity of the types of issues is not something you can dictate or infer from PyLint itself; it's something the owner of the Quality Profile in SonarQube has to configure. Hence I do not think it makes much sense for PyLint to be able to output SonarQube-compatible JSON solely by itself. You will need user input for this. Without configuring the severities, you might as well use the build-in pylint import functionality of SonarQube (which puts all issue severities to MAJOR).

Btw, the severity of PyLint issues used to be configurable from within SonarQube, but they removed this functionality in recent releases.

Ah, okay. Yeah, this is probably something that should stay in an external plugin. Thanks for contributing to the pylint network though! 😄

I'll leave this issue open because I still think that we should either update the documentation to reflect this or find a way to allow registering options more naturally.

DanielNoord commented 2 years ago

@teake I was wondering how much effort adding the additional checker was for you. I have thought of a couple of ways to allow creating options for Reporters but basically they all involve completely rewriting the inheritance pattern of Reporters which would be another breaking change for 3.0.

I'd like to avoid those where possible and I'm thinking of just stating that adding a bare checker is the preferred way to handle these cases.

teake commented 2 years ago

@DanielNoord IIRC it wasn't that much of a hassle -- the hassle was in trying to find a way to register options on reporters. If it's stated clearly in the docs that checkers are the way to do this (with some examples) I think that should be perfectly ok.