Typo in top level `pylintrc` and possible bug in `missing-docstring` or `useless-suppression`

[scop]

Bug description

The top level pylintrc for pylint itself has a typo:

@@ -54 +54 @@ enable=
-    useless-supression,
+    useless-suppression,

Fixing that typo causes a test suite failure, because of one new useless-suppression message:

 ______________ TestRunTC.test_json_report_does_not_escape_quotes _______________

self = <test_self.TestRunTC object at 0x7f13cd73b5f8>

    def test_json_report_does_not_escape_quotes(self) -> None:
        out = StringIO()
        module = join(HERE, "regrtest_data", "unused_variable.py")
        self._runtest([module], code=4, reporter=JSONReporter(out))
        output = json.loads(out.getvalue())
        assert isinstance(output, list)
>       assert len(output) == 1
E       assert 2 == 1
E        +  where 2 = len([{'column': 4, 'line': 4, 'message': "Unused variable 'variable'", 'message-id': 'W0612', ...}, {'column': 0, 'line': 1, 'message': "Useless suppression of 'missing-class-docstring'", 'message-id': 'I0021', ...}])

I'm reporting this here instead of filing a PR, in part because I think it's faster for someone who knows the preferred way to deal with this to just fix it instead of going back and forth with a PR..

But more importantly, the useless suppression message seems to indicate a pylint bug or at least behavior which is weird to me:

./tests/regrtest_data/unused_variable.py doesn't even contain a suppression for missing-class-docstring which is said to be useless. It just has one for missing-docstring. I wondered if this could be if the latter was a superset of the former or something, but then again even then the suppression would not seem useless because there just isn't a class docstring in the said file at all, so the warning about the suppression would seem spot on.

TODO

• Fix the typo and enable useless suppression
• Handle the "false positive" / working as intended for useless suppression of missing docstring

Pylint version

Current git main

[Pierre-Sassoulas]

Nice (double) catch !

missing-docstring was burst in multiple messages (my first pylint PR btw 😄) and is now the placeholder for f"missing-{x}-docstring" for x in ["class", "module", "function"]. Making that possible was a huge refactor where we had to handle a list of message instead of a single message, and it seems I missed the implication for useless-suppression . So I guess that when using missing-docstring there is a useless suppression for missing-module-docstring but I think this should change.

[DanielNoord]

Don't you think changing the pylint: disable in tests/regrtest_data/unused_variable.py will be better? Seems like quite a big effort to make missing-docstring not result in useless-suppression.
We would need to add code to the useless-suppression checker to see for each message that seems useless if 1) it has an old message, 2) if that old-message has other new messages, 3) if any of those other new messages is reported. Considering this change has been in pylint for quite some time I think it might be better to just change this and leave as is. If we get any other issues about old-messages generating useless-suppression we might reconsider.

[Pierre-Sassoulas]

There isn't a lot of old_names with multiple new name, in fact I think there's only missing-docstrings. I've started a fix for this, but I think I will merge the hot fix before I change the way useless-suppression works (the typing you added recentely would help btw).

[DanielNoord]

@Pierre-Sassoulas I just thought of this because of the recent comments in astroid. Have you created a branch for your fix? If I get the time I can take a look and might be able to finish this. Seems useful to fix this typo this in 2.12 or 2.13.

[Pierre-Sassoulas]

Sorry I've completely forgotten about this. I actually did not create a branch and had to pop everything from my git stash since 20th of September. I saw some things I can't speak of 😄 Anyway, I think I found the work in progress I had. It's still not ready at all but I pushed the branch here if you want to try to salvage something : https://github.com/Pierre-Sassoulas/pylint/tree/work-in-progress-5040.

[DanielNoord]

@Pierre-Sassoulas I have been looking at this for a couple of minutes and this would be pretty difficult to solve (I think).
We currently don't store the old message and its id with the ids it looks to now (or at least for C0111 I can't find it). That is, old names are created because they exist as attributes of new messages instead of an old message entry "birthing" new separate messages.

Therefore when we disable an old message and we want to see if it is being used we can't easily look up which messages should be emitted for it not to be useless.

However, I would like to make a case that we could also consider this a won't-fix or not false-positive. Because while missing-docstring is not useless for disabling missing-class-docstring it is for missing-function-docstring. Instead of finding a disable useful when any out of its new messages are being emitted you could also argue it is only useful when all messages are being emitted. Since no issues for this particular point have been opened keeping the status quo (which is essentially the all variant) doesn't seem too problematic. It also serves as a reminder to users to update messages to the new message name.
It would say we can make a feature out of a bug here 😄

[Pierre-Sassoulas]

There's probably a way to recover it by modifying the MessageIdStore, but that's a pretty minor issue I don't think it's worth it. Let's separate the two issues and reactivate useless-suppression first.