SONARPY-743 RSPEC-5864 Calling a non-callable type#828
SONARPY-743 RSPEC-5864 Calling a non-callable type#828andrea-guarino-sonarsource merged 3 commits intomasterfrom
Conversation
andrea-guarino-sonarsource
force-pushed
the
SONARPY-743
branch
9 times, most recently
from
b4c5ed8 to
5f6ab80
Compare
guillaume-dequenne
left a comment
guillaume-dequenne
left a comment
There was a problem hiding this comment.
lgtm, minor comments.
There was a problem hiding this comment.
I think you mean returns true here.
There was a problem hiding this comment.
Indeed :) Thanks!
There was a problem hiding this comment.
I'd rename to declaresMember to be consistent with the way we named the other methods.
There was a problem hiding this comment.
I named it like this to be consistent with resolveMember. I think we're already inconsistent (we have isCompatibleWith), not sure which one we should prefer
There was a problem hiding this comment.
I'm a bit confused by type.canHaveMember("__call__"). Is this a way of asserting that we're dealing with a DeclaredType? It feels a bit indirect and implementation dependent to me.
That said I'm not sure I have a suggestion for a more direct check (checking directly that type is an instance of DeclaredType should do the trick but it might be heavy-handed...but then again this rule is tailored for DeclaredType only so it might make sense)
There was a problem hiding this comment.
Yes, the idea is to avoid raising twice the same issue (one for the bug rule and one for code smell).
I think we cannot check only for type being instance of DeclaredType. It could also be a UnionType containing DeclaredType and we still want to raise an issue.
I agree it's a bit indirect; if you agree I'll add a comment explaining this
There was a problem hiding this comment.
Following the fix on copyWithoutUsages, I think you can remove this.
There was a problem hiding this comment.
Not sure we should consider this a FN. We should raise a code smell for the impossible + operation above, though, then we shouldn't be able to resolve a type for z I assume.
I believe this one would be a FN though (we should infer a DeclaredType of str for y):
def foo(x: str):
y = x + "hello"
y() # FN
There was a problem hiding this comment.
yes indeed, I agree
andrea-guarino-sonarsource
force-pushed
the
SONARPY-743
branch
from
5f6ab80 to
a22b281
Compare
|
Kudos, SonarQube Quality Gate passed! |
GitOrigin-RevId: afe2cf9a7e9b8d88d79321adda5bc9aa2bdbb4b3
3 participants
No description provided.