Emit a correct invalid-name message when using multi-naming style. (#4924)
* Emit a correct invalid-name message when using multi-naming style.
Previously, given `--function-rgx=(?:(?P<UP>[A-Z]+)|(?P<down>[a-z]+))$` and the code:
```
def FOO():
pass
def UPPER():
pass
def lower():
pass
```
It would emit a message: ``Function name `lower` doesn't conform to '(?:(?P<UP>[A-Z]+)|(?P<down>[a-z]+))$' pattern [invalid-name]``.
The message is misleading as `lower` *does* conform to `(?:(?P<UP>[A-Z]+)|(?P<down>[a-z]+))$`. It's just not the prevalent group "UP".
After this commit, the message becomes: ``Function name `lower` doesn't conform to UP group in the '(?:(?P<UP>[A-Z]+)|(?P<down>[a-z]+))$' pattern [invalid-name]``
Co-authored-by: Pierre Sassoulas <[email protected]>diff --git a/ChangeLog b/ChangeLog
index 2664a84..f0f5c1a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -43,6 +43,7 @@
..
Put bug fixes that should not wait for a new minor version here
+* The ``invalid-name`` message is now more detailed when using multiple naming style regexes.
What's New in Pylint 2.10.2?
diff --git a/doc/whatsnew/2.11.rst b/doc/whatsnew/2.11.rst
index bd95563..ce66052 100644
--- a/doc/whatsnew/2.11.rst
+++ b/doc/whatsnew/2.11.rst
@@ -40,3 +40,4 @@
* Added ``py-version`` config key (if ``[MASTER]`` section). Used for version dependant checks.
Will default to whatever Python version pylint is executed with.
+* The ``invalid-name`` message is now more detailed when using multiple naming style regexes.
diff --git a/pylint/checkers/base.py b/pylint/checkers/base.py
index 6aba62d..d2599d9 100644
--- a/pylint/checkers/base.py
+++ b/pylint/checkers/base.py
@@ -66,7 +66,7 @@
import itertools
import re
import sys
-from typing import Pattern
+from typing import Optional, Pattern
import astroid
from astroid import nodes
@@ -1903,6 +1903,7 @@
continue
groups = collections.defaultdict(list)
min_warnings = sys.maxsize
+ prevalent_group, _ = max(all_groups.items(), key=lambda item: len(item[1]))
for group in all_groups.values():
groups[len(group)].append(group)
min_warnings = min(len(group), min_warnings)
@@ -1915,7 +1916,7 @@
else:
warnings = groups[min_warnings][0]
for args in warnings:
- self._raise_name_warning(*args)
+ self._raise_name_warning(*args, prevalent_group=prevalent_group)
@utils.check_messages(
"disallowed-name", "invalid-name", "assign-to-new-keyword", "non-ascii-name"
@@ -2016,10 +2017,21 @@
return self._name_group.get(node_type, node_type)
def _raise_name_warning(
- self, node, node_type, name, confidence, warning="invalid-name"
- ):
+ self,
+ node: nodes.NodeNG,
+ node_type: str,
+ name: str,
+ confidence,
+ warning: str = "invalid-name",
+ prevalent_group: Optional[str] = None,
+ ) -> None:
type_label = HUMAN_READABLE_TYPES[node_type]
hint = self._name_hints[node_type]
+ if prevalent_group:
+ # This happens in the multi naming match case. The expected
+ # prevalent group needs to be spelled out to make the message
+ # correct.
+ hint = f"the `{prevalent_group}` group in the {hint}"
if self.config.include_naming_hint:
hint += f" ({self._name_regexps[node_type].pattern!r} pattern)"
args = (
diff --git a/tests/checkers/unittest_base.py b/tests/checkers/unittest_base.py
index 7121f25..6a20db9 100644
--- a/tests/checkers/unittest_base.py
+++ b/tests/checkers/unittest_base.py
@@ -303,7 +303,11 @@
message = Message(
"invalid-name",
node=classes[0],
- args=("Class", "classb", "'(?:(?P<UP>[A-Z]+)|(?P<down>[a-z]+))$' pattern"),
+ args=(
+ "Class",
+ "classb",
+ "the `UP` group in the '(?:(?P<UP>[A-Z]+)|(?P<down>[a-z]+))$' pattern",
+ ),
)
with self.assertAddsMessages(message):
cls = None
@@ -340,7 +344,7 @@
args=(
"Class",
"CLASSC",
- "'(?:(?P<UP>[A-Z]+)|(?P<down>[a-z]+))$' pattern",
+ "the `down` group in the '(?:(?P<UP>[A-Z]+)|(?P<down>[a-z]+))$' pattern",
),
),
]
@@ -371,7 +375,11 @@
message = Message(
"invalid-name",
node=function_defs[1],
- args=("Function", "FUNC", "'(?:(?P<UP>[A-Z]+)|(?P<down>[a-z]+))$' pattern"),
+ args=(
+ "Function",
+ "FUNC",
+ "the `down` group in the '(?:(?P<UP>[A-Z]+)|(?P<down>[a-z]+))$' pattern",
+ ),
)
with self.assertAddsMessages(message):
func = None
@@ -402,7 +410,7 @@
args=(
"Function",
"UPPER",
- "'(?:(?P<ignore>FOO)|(?P<UP>[A-Z]+)|(?P<down>[a-z]+))$' pattern",
+ "the `down` group in the '(?:(?P<ignore>FOO)|(?P<UP>[A-Z]+)|(?P<down>[a-z]+))$' pattern",
),
)
with self.assertAddsMessages(message):
diff --git a/tests/functional/i/invalid/invalid_name_multinaming_style.py b/tests/functional/i/invalid/invalid_name_multinaming_style.py
new file mode 100644
index 0000000..561a154
--- /dev/null
+++ b/tests/functional/i/invalid/invalid_name_multinaming_style.py
@@ -0,0 +1,13 @@
+"""Test for multi naming style."""
+
+
+def UPPER(): # [invalid-name]
+ """UPPER func that will generate invalid-name error."""
+
+
+def lower():
+ """lower func that satisfies the prevalent group `lower`."""
+
+
+def anotherlower():
+ """Another lower func that satisfies the prevalent group `lower`."""
diff --git a/tests/functional/i/invalid/invalid_name_multinaming_style.rc b/tests/functional/i/invalid/invalid_name_multinaming_style.rc
new file mode 100644
index 0000000..d513cfb
--- /dev/null
+++ b/tests/functional/i/invalid/invalid_name_multinaming_style.rc
@@ -0,0 +1,2 @@
+[BASIC]
+function-rgx=^(?:(?P<UP>[A-Z]+)|(?P<down>[a-z]+))$
diff --git a/tests/functional/i/invalid/invalid_name_multinaming_style.txt b/tests/functional/i/invalid/invalid_name_multinaming_style.txt
new file mode 100644
index 0000000..abcde05
--- /dev/null
+++ b/tests/functional/i/invalid/invalid_name_multinaming_style.txt
@@ -0,0 +1 @@
+invalid-name:4:0:UPPER:"Function name ""UPPER"" doesn't conform to the `down` group in the '^(?:(?P<UP>[A-Z]+)|(?P<down>[a-z]+))$' pattern":HIGH
