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.

Sign up for GitHub

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

Always use the suggestion mode for no-member / c-extension-no-member #9962

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
+27 −99
Open

Always use the suggestion mode for no-member / c-extension-no-member #9962

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
51c9061
[suggestion-mode] Remove the option and always suggest
Pierre-Sassoulas May 4, 2023
8e103a6
_is_c_extension already checks if owner is a Module
Pierre-Sassoulas Dec 1, 2024
2d1c86e
Upgrade doc
Pierre-Sassoulas Dec 1, 2024
17a9dca
Add changelog and remove outdated tests
Pierre-Sassoulas Dec 1, 2024
fd026a1
Update doc/whatsnew/fragments/9962.breaking
Pierre-Sassoulas Dec 2, 2024
d2abf8d
Update pylint/checkers/typecheck.py
Pierre-Sassoulas Dec 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 0 additions & 9 deletions doc/user_guide/configuration/all-options.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,13 +209,6 @@ Standard Checkers
**Default:** ``()``


--suggestion-mode
"""""""""""""""""
*When enabled, pylint would attempt to guess common misconfiguration and emit user-friendly hints instead of false-positive error messages.*

**Default:** ``True``


--unsafe-load-any-extension
"""""""""""""""""""""""""""
*Allow loading of arbitrary C extensions. Extensions are imported into the active Python interpreter and may run arbitrary code.*
Expand Down Expand Up @@ -290,8 +283,6 @@ Standard Checkers

source-roots = []

suggestion-mode = true

unsafe-load-any-extension = false


Expand Down
4 changes: 4 additions & 0 deletions doc/whatsnew/fragments/9962.breaking
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
The ``suggestion-mode`` option was removed, pylint now always try to emit user-friendly hints instead
of false-positive error messages. You should remove it from your conf, if it's defined.
Pierre-Sassoulas marked this conversation as resolved.
Show resolved Hide resolved

Refs #9962
7 changes: 3 additions & 4 deletions examples/pylintrc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,6 @@ recursive=no
# source root.
source-roots=

# When enabled, pylint would attempt to guess common misconfiguration and emit
# user-friendly hints instead of false-positive error messages.
suggestion-mode=yes

# Allow loading of arbitrary C extensions. Extensions are imported into the
# active Python interpreter and may run arbitrary code.
unsafe-load-any-extension=no
Expand Down Expand Up @@ -456,6 +452,9 @@ timeout-methods=requests.api.delete,requests.api.get,requests.api.head,requests.

[MISCELLANEOUS]

# Whether or not to search for fixme's in docstrings.
check-fixme-in-docstring=no

# List of note tags to take in consideration, separated by a comma.
notes=FIXME,
XXX,
Expand Down
7 changes: 3 additions & 4 deletions examples/pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,6 @@ py-version = "3.12"
# source root.
# source-roots =

# When enabled, pylint would attempt to guess common misconfiguration and emit
# user-friendly hints instead of false-positive error messages.
suggestion-mode = true

# Allow loading of arbitrary C extensions. Extensions are imported into the
# active Python interpreter and may run arbitrary code.
# unsafe-load-any-extension =
Expand Down Expand Up @@ -387,6 +383,9 @@ disable = ["raw-checker-failed", "bad-inline-option", "locally-disabled", "file-
timeout-methods = ["requests.api.delete", "requests.api.get", "requests.api.head", "requests.api.options", "requests.api.patch", "requests.api.post", "requests.api.put", "requests.api.request"]

[tool.pylint.miscellaneous]
# Whether or not to search for fixme's in docstrings.
# check-fixme-in-docstring =

# List of note tags to take in consideration, separated by a comma.
notes = ["FIXME", "XXX", "TODO"]

Expand Down
56 changes: 16 additions & 40 deletions pylint/checkers/typecheck.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,26 +201,6 @@ def _similar_names(
return sorted(picked)


def _missing_member_hint(
owner: SuccessfulInferenceResult,
attrname: str | None,
distance_threshold: int,
max_choices: int,
) -> str:
names = _similar_names(owner, attrname, distance_threshold, max_choices)
if not names:
# No similar name.
return ""

names = [repr(name) for name in names]
if len(names) == 1:
names_hint = ", ".join(names)
else:
names_hint = f"one of {', '.join(names[:-1])} or {names[-1]}"

return f"; maybe {names_hint}?"


MSGS: dict[str, MessageDefinitionTuple] = {
"E1101": (
"%s %r has no %r member%s",
Expand Down Expand Up @@ -984,10 +964,6 @@ def open(self) -> None:
self._py310_plus = py_version >= (3, 10)
self._mixin_class_rgx = self.linter.config.mixin_class_rgx

@cached_property
def _suggestion_mode(self) -> bool:
return self.linter.config.suggestion_mode # type: ignore[no-any-return]

@cached_property
def _compiled_generated_members(self) -> tuple[Pattern[str], ...]:
# do this lazily since config not fully initialized in __init__
Expand Down Expand Up @@ -1198,24 +1174,24 @@ def _get_nomember_msgid_hint(
node: nodes.Attribute | nodes.AssignAttr | nodes.DelAttr,
owner: SuccessfulInferenceResult,
) -> tuple[Literal["c-extension-no-member", "no-member"], str]:
suggestions_are_possible = self._suggestion_mode and isinstance(
owner, nodes.Module
if _is_c_extension(owner):
return "c-extension-no-member", ""
if not self.linter.config.missing_member_hint:
return "no-member", ""
names = _similar_names(
Copy link
Member

@jacobtylerwalls jacobtylerwalls Oct 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Could you share a profile run for a project like pylint or astroid and let us know the total time spent in _similar_names? That might impact whether we want to lru_cache it, potentially after fiddling with when attname is filtered out. Let me know if I'm speaking in too much shorthand.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hurgh, took ~=50mn to lint pandas (I keyboard interrupted), but I didn't manage to format the output properly for snakeviz so no cool pictures available, but anyway the info about _similar_names:

   Ordered by: cumulative time

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
         1    0.000    0.000 2997.168 2997.168 __main__.py:1(<module>)
...
      571    0.952    0.002   85.079    0.149 typecheck.py:171(_similar_names)

Looks like it's 2.80% of total time, not insignificant.. (50% of the time seems to be spent in calculating string distances).

Copy link
Member

Choose a reason for hiding this comment

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

Thanks. I also just tried with django, a codebase where I expect no-member would fire a lot (hence pylint-django) and got similar results.

Without caching:
1.923s out of 44.409s total (4.3% of time)

With lru_cache():
CacheInfo(hits=168, misses=239, maxsize=512, currsize=239)
1.328s out of 43.122s total (3% of time)

Seems worth it? Maybe a maxsize of 256 or so?

Copy link
Member Author

Choose a reason for hiding this comment

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

I would have expected the cache to do better than that. Did you use the default ? And on what function ? Maybe it's more impactful to cache the string distance function than the function with the information about the instance. We definitely should cache in any case. (Also, you seem to have a vastly better computer than mine haha, linting Django in 45s ! :star-eyes:)

Copy link
Member

Choose a reason for hiding this comment

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

Did you use the default ?

I increased to maxsize=512, but as you can see the cache only ever grew to 239. Makes sense, there will only ever be so many no-member errors in a mature code base.

And on what function ?

_similar_names()

owner,
node.attrname,
self.linter.config.missing_member_hint_distance,
self.linter.config.missing_member_max_choices,
)
if suggestions_are_possible and _is_c_extension(owner):
msg = "c-extension-no-member"
hint = ""
if not names:
return "no-member", ""
names = [repr(name) for name in names]
if len(names) == 1:
names_hint = ", ".join(names)
Pierre-Sassoulas marked this conversation as resolved.
Show resolved Hide resolved
else:
msg = "no-member"
if self.linter.config.missing_member_hint:
hint = _missing_member_hint(
owner,
node.attrname,
self.linter.config.missing_member_hint_distance,
self.linter.config.missing_member_max_choices,
)
else:
hint = ""
return msg, hint # type: ignore[return-value]
names_hint = f"one of {', '.join(names[:-1])} or {names[-1]}"
return "no-member", f"; maybe {names_hint}?"

@only_required_for_messages(
"assignment-from-no-return",
Expand Down
13 changes: 0 additions & 13 deletions pylint/lint/base_options.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,19 +306,6 @@ def _make_linter_options(linter: PyLinter) -> Options:
),
},
),
(
"suggestion-mode",
{
"type": "yn",
"metavar": "<y or n>",
"default": True,
"help": (
"When enabled, pylint would attempt to guess common "
"misconfiguration and emit user-friendly hints instead "
"of false-positive error messages."
),
},
),
(
"exit-zero",
{
Expand Down
1 change: 0 additions & 1 deletion pylint/utils/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@

# These are types used to overload get_global_option() and refer to the options type
GLOBAL_OPTION_BOOL = Literal[
"suggestion-mode",
"analyse-fallback-blocks",
"allow-global-unused-variables",
"prefer-stubs",
Expand Down
4 changes: 0 additions & 4 deletions pylintrc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,6 @@ load-plugins=
# number of processors available to use.
jobs=1

# When enabled, pylint would attempt to guess common misconfiguration and emit
# user-friendly hints instead of false-positive error messages.
suggestion-mode=yes

# Allow loading of arbitrary C extensions. Extensions are imported into the
# active Python interpreter and may run arbitrary code.
unsafe-load-any-extension=no
Expand Down
25 changes: 1 addition & 24 deletions tests/checkers/unittest_typecheck.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

from pylint.checkers import typecheck
from pylint.interfaces import INFERENCE, UNDEFINED
from pylint.testutils import CheckerTestCase, MessageTest, set_config
from pylint.testutils import CheckerTestCase, MessageTest

try:
from coverage import tracer as _
Expand All @@ -27,29 +27,6 @@ class TestTypeChecker(CheckerTestCase):

CHECKER_CLASS = typecheck.TypeChecker

@set_config(suggestion_mode=False)
@needs_c_extension
def test_nomember_on_c_extension_error_msg(self) -> None:
node = astroid.extract_node(
"""
from coverage import tracer
tracer.CTracer #@
"""
)
message = MessageTest(
"no-member",
node=node,
args=("Module", "coverage.tracer", "CTracer", ""),
confidence=INFERENCE,
line=3,
col_offset=0,
end_line=3,
end_col_offset=14,
)
with self.assertAddsMessages(message):
self.checker.visit_attribute(node)

@set_config(suggestion_mode=True)
@needs_c_extension
def test_nomember_on_c_extension_info_msg(self) -> None:
node = astroid.extract_node(
Expand Down
Loading