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(script): Support label-tool-skip directive labels #4274

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

whisperity
Copy link
Contributor

@whisperity whisperity commented Jun 24, 2024

Important

⛔ Blocked by #4225.

In some cases, such as in #4243, it might be deemed necessary that a label (e.g., a higher or lower severity) is more appropriate than what the tool would auto-generate. In such cases, a label-tool-skip:LABEL_KEY directive can be applied, but prior to this patch, the tool disregarded this information. This patch implements the necessary handling for the existing aspects of the label-tool to appropriately respect these skip directives.

@whisperity whisperity added enhancement 🌟 tools 🛠️ Meta-tag for all the additional tools supplied with CodeChecker: plist2html, tu_collector, etc. config ⚙️ labels Jun 24, 2024
@whisperity whisperity added this to the release 6.25.0 milestone Jun 24, 2024
@whisperity whisperity requested a review from dkrupp June 24, 2024 15:06
@whisperity whisperity force-pushed the feat/script/label-tool/key-skipping branch from 9fc110d to 03f01ac Compare June 24, 2024 15:16
@whisperity whisperity added the label-tool 🔖 Related to tooling that manages the analyzer/checker label configuration label Jun 25, 2024
@whisperity whisperity force-pushed the feat/script/label-tool/key-skipping branch 2 times, most recently from d3507fc to 4042866 Compare June 25, 2024 14:08
@whisperity whisperity added the RDY-OnHold 🛑 Patch reviewed and ready, but don't merge due to having to merge a dependent patch first. label Jun 30, 2024
@whisperity whisperity marked this pull request as ready for review July 8, 2024 16:37
Automatically generate the `doc_url` labels for checkers for analysers
which we know how to do this.
(Currently implemented only for Clang SA and Clang-Tidy.)
This tooling does a single HTTP request to download a "Table of
Contents" (ToC) document and uses HTML DOM scraping to extract the list
of checkers and their corresponding documentation link.

There was existing prior work for this feature, but those scripts were
about 3 years old (introduced in 2021. Nov, commit
aa72dc0), and they ceased to properly
work.
For example, for _Clang-Tidy_, the contents of the ToC changed in a
way that the previously used XPath expression did not match anything
at all.
In addition, the previous work at `doc_url.py` (later renamed
`doc_url_generate.py`) only generated labels for the checkers that were
_already_ present in the configuration file, defeating the purpose of
using a script to determine new labels for checkers.

This version reintroduces an improved DOM scraping logic, and enables
generating `doc_url` for checkers that are not yet present in the
configuration file, all the while using the new `label_tool/` package
structure introduced in a previous commit.
Automatically generate the `severity` labels for checkers of analysers
which we know how to.
(Currently implemented only for the Clang diagnostic (warning) flags
exposed through Clang-Tidy.)

There was existing prior work for this feature, but that script was
about 3 years old (introduced in 2021. Nov, commit
8d1a7fe), and due to changes in the
DOM of Sphinx documents, ceased to work properly since.

This refactoring is smaller in scope than the `doc_url` generation that
was done in a previous commit, as the original `compiler_warnings.py`
script accurately generated severities (and `doc_url`s, for that matter)
for previously "unknown" "checkers" as well.
Considering that all previously developed label-generating tools are now
subsumed into the new `label_tool` package, the extra directory is unnecessary.
@whisperity whisperity force-pushed the feat/script/label-tool/key-skipping branch from 4042866 to 962f449 Compare July 9, 2024 17:07
After upgrade on `master` to `pylint==3.2.4`, new warnings are enabled
which were not honoured in `label-tool`.
In some cases it might be deemed necessary that a label (e.g., a higher
or lower `severity`) is more appropriate than what the tool would
otherwise auto-generate.
In such cases, a `label-tool-skip:LABEL_KEY` (e.g.,
`label-tool-skip:severity`, as applied in
commit 04d27ab) can indicate to stamp
the fact that a developer overruled the tools' decisions.

Prior to this commit, the tooling actually disregarded this information,
but now I added the necessary handling to the existing aspects.
@whisperity whisperity force-pushed the feat/script/label-tool/key-skipping branch from 962f449 to 6c22160 Compare July 10, 2024 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
config ⚙️ enhancement 🌟 label-tool 🔖 Related to tooling that manages the analyzer/checker label configuration RDY-OnHold 🛑 Patch reviewed and ready, but don't merge due to having to merge a dependent patch first. tools 🛠️ Meta-tag for all the additional tools supplied with CodeChecker: plist2html, tu_collector, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant