Skip to content

Colorize "fail-on" messages/categories in ColorizedTextReporter red inverse #10444

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
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

felixp98
Copy link

@felixp98 felixp98 commented Jun 29, 2025

Type of Changes

Type
πŸ› Bug fix
βœ“ ✨ New feature
πŸ”¨ Refactoring
πŸ“œ Docs

Description

colorized reporter now colorizes messages/categories that have been configured as fail-on in red inverse.
This makes it easier to quickly find the errors that are causing pylint CI job failures.

Example

python code to be linted
import os

def example():
    try:
        pass
    except Exception:
        pass
    except OSError:
        pass

with following fail-on configuration:
fail-on=E, missing-function-docstring

results in:
grafik


Note on the changes

I'm not sure if there is a better way to pass the configured fail-on symbols to the ColorizedTextReporter class.
At the time at which the constructor of the reporter class is called, the fail_on symbols have not been parsed/converted to specific msgs. Therefore I'm passing the fail_on_symbol list to the reporter afterwards.
I thought about adding a fail_on bool parameter to the Message class to avoid passing the fail_on_symbol list to the reporter class, but I think that would have too many impacts on other areas/reporters, e.g. the json output.


Closes #9898

@Pierre-Sassoulas Pierre-Sassoulas added the Enhancement ✨ Improvement to a component label Jun 29, 2025
@Pierre-Sassoulas Pierre-Sassoulas added this to the 4.0.0 milestone Jun 29, 2025
Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

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

This look pretty good, without looking too much into it the only problem I see could be adding more to the pylinter. I'd rather have the logic be in the reporter itself, but maybe that's not possible.

Copy link

codecov bot commented Jun 29, 2025

Codecov Report

Attention: Patch coverage is 73.33333% with 4 lines in your changes missing coverage. Please review.

Project coverage is 95.86%. Comparing base (0707127) to head (58a4953).

Files with missing lines Patch % Lines
pylint/lint/pylinter.py 62.50% 3 Missing ⚠️
pylint/reporters/text.py 83.33% 1 Missing ⚠️

❌ Your patch check has failed because the patch coverage (73.33%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #10444      +/-   ##
==========================================
- Coverage   95.88%   95.86%   -0.02%     
==========================================
  Files         176      176              
  Lines       19140    19153      +13     
==========================================
+ Hits        18352    18361       +9     
- Misses        788      792       +4     
Files with missing lines Coverage Ξ”
pylint/config/config_initialization.py 98.91% <100.00%> (+0.01%) ⬆️
pylint/reporters/text.py 94.32% <83.33%> (-0.53%) ⬇️
pylint/lint/pylinter.py 96.39% <62.50%> (-0.53%) ⬇️
πŸš€ New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

πŸ€– Effect of this PR on checked open source code: πŸ€–

Effect on home-assistant:
The following messages are now emitted:

  1. import-private-name:
    Imported private module (_collections_abc)
    https://github.com/home-assistant/core/blob/e210681751249c1b92fb74adfdaa72baf9a4ccc8/homeassistant/components/smlight/binary_sensor.py#L5

Effect on music21:
The following messages are now emitted:

  1. invalid-name:
    Attribute name "id" doesn't conform to '[a-z_][A-Za-z0-9_]{2,30}$' pattern
    https://github.com/cuthbertLab/music21/blob/f2e09f14916ece7651122d160a43b8950608182b/music21/base.py#L612
  2. redefined-variable-type:
    Redefinition of sc3 type from music21.scale.MinorScale to music21.scale.MajorScale
    https://github.com/cuthbertLab/music21/blob/f2e09f14916ece7651122d160a43b8950608182b/music21/scale/test_scale_main.py#L163

The following messages are no longer emitted:

  1. invalid-name:
    Attribute name "id" doesn't conform to '[a-z_][A-Za-z0-9_]{2,30}$' pattern
    https://github.com/cuthbertLab/music21/blob/f2e09f14916ece7651122d160a43b8950608182b/music21/prebase.py#L293
  2. redefined-variable-type:
    Redefinition of sc3 type from music21.scale.MajorScale to music21.scale.MinorScale
    https://github.com/cuthbertLab/music21/blob/f2e09f14916ece7651122d160a43b8950608182b/music21/scale/test_scale_main.py#L169

Effect on psycopg:
The following messages are now emitted:

  1. c-extension-no-member:
    Module 'psycopg.pq._pq_ctypes' has no 'PQnoticeReceiver' member, but source is unavailable. Consider adding this module to extension-pkg-allow-list if you want to perform analysis based on run-time introspection of living objects.
    https://github.com/psycopg/psycopg/blob/ec15df110bcc25a63099cd22731f3b4bdd01cbb0/psycopg/psycopg/pq/pq_ctypes.py#L55
  2. too-many-function-args:
    Too many positional arguments for function call
    https://github.com/psycopg/psycopg/blob/ec15df110bcc25a63099cd22731f3b4bdd01cbb0/psycopg/psycopg/pq/pq_ctypes.py#L360

The following messages are no longer emitted:

  1. unreachable:
    Unreachable code
    https://github.com/psycopg/psycopg/blob/ec15df110bcc25a63099cd22731f3b4bdd01cbb0/psycopg/psycopg/pq/pq_ctypes.py#L545
  2. unreachable:
    Unreachable code
    https://github.com/psycopg/psycopg/blob/ec15df110bcc25a63099cd22731f3b4bdd01cbb0/psycopg/psycopg/pq/pq_ctypes.py#L562
  3. unreachable:
    Unreachable code
    https://github.com/psycopg/psycopg/blob/ec15df110bcc25a63099cd22731f3b4bdd01cbb0/psycopg/psycopg/pq/pq_ctypes.py#L622
  4. unreachable:
    Unreachable code
    https://github.com/psycopg/psycopg/blob/ec15df110bcc25a63099cd22731f3b4bdd01cbb0/psycopg/psycopg/pq/pq_ctypes.py#L712
  5. unreachable:
    Unreachable code
    https://github.com/psycopg/psycopg/blob/ec15df110bcc25a63099cd22731f3b4bdd01cbb0/psycopg/psycopg/pq/pq_ctypes.py#L730
  6. unreachable:
    Unreachable code
    https://github.com/psycopg/psycopg/blob/ec15df110bcc25a63099cd22731f3b4bdd01cbb0/psycopg/psycopg/pq/pq_ctypes.py#L773
  7. unreachable:
    Unreachable code
    https://github.com/psycopg/psycopg/blob/ec15df110bcc25a63099cd22731f3b4bdd01cbb0/psycopg/psycopg/pq/pq_ctypes.py#L990

This comment was generated for commit 58a4953

@felixp98
Copy link
Author

This look pretty good, without looking too much into it the only problem I see could be adding more to the pylinter. I'd rather have the logic be in the reporter itself, but maybe that's not possible.

I mean, the logic is in the reporter, but somehow the reporter has to get the configured fail_on_symbols from the PyLinter instance. If you have an idea on how to do it in a cleaner way than my approach, I'd be happy to do it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement ✨ Improvement to a component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dynamic color mapping for "fail-on" messages/categories in ColorizedTextReporter
2 participants