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

fix false negative when isinstance is provided too many or too few arguments #9868

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions doc/data/messages/t/too-few-function-args/bad.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
isinstance("foo") # [too-few-function-args]
Copy link
Member

Choose a reason for hiding this comment

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

Something minor for next time: in the pylint docs we tend to avoid "foo" and "bar" because junior programmers are often the target audience, and if they don't know the convention/backstory it has the potential to be distracting.

Also, "foo" is a literal just like 1 -- I was suggesting to change it for the reason that if we introduce a check later like literal-isinstance (in the spirit of using-constant-test), then we'll have to adjust this example. But let's not change it now, I'm not sure it's worth fleshing out this example more.

2 changes: 2 additions & 0 deletions doc/data/messages/t/too-few-function-args/details.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
As of 2024-08-13, #9847, ```too-few-function-args``` is only implemented for the
```isinstance``` built-in function.
1 change: 1 addition & 0 deletions doc/data/messages/t/too-few-function-args/good.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
isinstance("foo", str)
2 changes: 2 additions & 0 deletions doc/user_guide/checkers/features.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1241,6 +1241,8 @@ Typecheck checker Messages
:invalid-slice-step (E1144): *Slice step cannot be 0*
Used when a slice step is 0 and the object doesn't implement a custom
__getitem__ method.
:too-few-function-args (E1145): *Too few positional arguments for %s call*
Used when a function or method call has fewer arguments than expected.
:too-many-function-args (E1121): *Too many positional arguments for %s call*
Used when a function call passes too many positional arguments.
:unexpected-keyword-arg (E1123): *Unexpected keyword argument %r in %s call*
Expand Down
1 change: 1 addition & 0 deletions doc/user_guide/messages/messages_overview.rst
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ All messages in the error category:
error/star-needs-assignment-target
error/syntax-error
error/too-few-format-args
error/too-few-function-args
error/too-many-format-args
error/too-many-function-args
error/too-many-star-expressions
Expand Down
5 changes: 5 additions & 0 deletions doc/whatsnew/fragments/9847.false_negative
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Fix a false negative when `isinstance` has too many arguments.
rogersheu marked this conversation as resolved.
Show resolved Hide resolved
Change now emits a `too-many-function-args` output with behavior similar to other
`too-many-function-args` calls.

Closes #9847
5 changes: 5 additions & 0 deletions doc/whatsnew/fragments/9847.new_check
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Add `too-few-function-args` for when fewer arguments than expected are provided for a
function or method call.
As of this PR, `too-few-function-args` only applies to the built-in function `isinstance`.

Closes #9847
30 changes: 25 additions & 5 deletions pylint/checkers/typecheck.py
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,11 @@ def _missing_member_hint(
"Used when a slice step is 0 and the object doesn't implement "
"a custom __getitem__ method.",
),
"E1145": (
"Too few positional arguments for %s call",
"too-few-function-args",
"Used when a function or method call has fewer arguments than expected.",
),
"W1113": (
"Keyword argument before variable positional arguments list "
"in the definition of %s function",
Expand Down Expand Up @@ -1419,9 +1424,22 @@ def _check_argument_order(
if calling_parg_names != called_param_names[: len(calling_parg_names)]:
self.add_message("arguments-out-of-order", node=node, args=())

def _check_isinstance_args(self, node: nodes.Call) -> None:
if len(node.args) != 2:
# isinstance called with wrong number of args
def _check_isinstance_args(self, node: nodes.Call, callable_name: str) -> None:
if len(node.args) > 2:
# for when isinstance called with too many args
self.add_message(
"too-many-function-args",
node=node,
args=(callable_name,),
confidence=HIGH,
)
rogersheu marked this conversation as resolved.
Show resolved Hide resolved
elif len(node.args) < 2:
self.add_message(
"too-few-function-args",
node=node,
args=(callable_name,),
confidence=HIGH,
)
return

second_arg = node.args[1]
Expand Down Expand Up @@ -1451,7 +1469,7 @@ def visit_call(self, node: nodes.Call) -> None:
if called.args.args is None:
if called.name == "isinstance":
# Verify whether second argument of isinstance is a valid type
self._check_isinstance_args(node)
self._check_isinstance_args(node, callable_name)
# Built-in functions have no argument information.
return

Expand Down Expand Up @@ -1554,7 +1572,9 @@ def visit_call(self, node: nodes.Call) -> None:
elif not overload_function:
# Too many positional arguments.
self.add_message(
"too-many-function-args", node=node, args=(callable_name,)
"too-many-function-args",
node=node,
args=(callable_name,),
)
break

Expand Down
4 changes: 2 additions & 2 deletions tests/functional/c/consider/consider_merging_isinstance.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ def isinstances():
result = isinstance(var[10], str) or isinstance(var[10], int) and var[8] * 14 or isinstance(var[10], float) and var[5] * 14.4 or isinstance(var[10], list) # [consider-merging-isinstance]
result = isinstance(var[11], int) or isinstance(var[11], int) or isinstance(var[11], float) # [consider-merging-isinstance]

result = isinstance(var[20])
result = isinstance()
result = isinstance(var[20]) # [too-few-function-args]
result = isinstance() # [too-few-function-args]

# Combination merged and not merged
result = isinstance(var[12], (int, float)) or isinstance(var[12], list) # [consider-merging-isinstance]
Expand Down
2 changes: 2 additions & 0 deletions tests/functional/c/consider/consider_merging_isinstance.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,6 @@ consider-merging-isinstance:19:13:19:73:isinstances:Consider merging these isins
consider-merging-isinstance:22:13:22:127:isinstances:Consider merging these isinstance calls to isinstance(var[6], (float, int)):UNDEFINED
consider-merging-isinstance:23:13:23:158:isinstances:Consider merging these isinstance calls to isinstance(var[10], (list, str)):UNDEFINED
consider-merging-isinstance:24:13:24:95:isinstances:Consider merging these isinstance calls to isinstance(var[11], (float, int)):UNDEFINED
too-few-function-args:26:13:26:32:isinstances:Too few positional arguments for function call:HIGH
too-few-function-args:27:13:27:25:isinstances:Too few positional arguments for function call:HIGH
consider-merging-isinstance:30:13:30:75:isinstances:Consider merging these isinstance calls to isinstance(var[12], (float, int, list)):UNDEFINED
3 changes: 3 additions & 0 deletions tests/functional/t/too/too_few_function_args.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# pylint: disable=missing-docstring

isinstance(1) # [too-few-function-args]
1 change: 1 addition & 0 deletions tests/functional/t/too/too_few_function_args.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
too-few-function-args:3:0:3:13::Too few positional arguments for function call:HIGH
5 changes: 5 additions & 0 deletions tests/functional/t/too/too_many_function_args.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,8 @@ def main(param):
if param == 0:
tmp = add
return tmp(1, 1.01)


# Negative case, see `_check_isinstance_args` in `./pylint/checkers/typecheck.py`
isinstance(1, int, int) # [too-many-function-args]
isinstance(1, 1, int) # [too-many-function-args, isinstance-second-argument-not-valid-type]
3 changes: 3 additions & 0 deletions tests/functional/t/too/too_many_function_args.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
too-many-function-args:23:0:23:23::Too many positional arguments for function call:HIGH
isinstance-second-argument-not-valid-type:24:0:24:21::Second argument of isinstance is not a type:INFERENCE
too-many-function-args:24:0:24:21::Too many positional arguments for function call:HIGH
Loading