Skip to content
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
category: minorAnalysis
---

- The call graph resolution no longer considers methods marked using [`@typing.overload`](https://typing.python.org/en/latest/spec/overload.html#overloads) as valid targets. This ensures that only the method that contains the actual implementation gets resolved as a target.
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,25 @@ predicate hasContextmanagerDecorator(Function func) {
)
}

/**
* Holds if the function `func` has a `typing.overload` decorator.
* Such functions are type stubs that declare an overload signature but are
* not the actual implementation.
*
* Normally we would want to model this using API graphs for more precision, but since this
* predicate is used in the call graph computation, we have to use a more syntactic approach.
*/
overlay[local]
private predicate hasOverloadDecorator(Function func) {
exists(ControlFlowNode overload |
overload.(NameNode).getId() = "overload" and overload.(NameNode).isGlobal()
or
overload.(AttrNode).getObject("overload").(NameNode).isGlobal()
Comment on lines +318 to +320
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume you need these two cases because you need to handle both @typing.overload and @overload?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's exactly right. Under normal circumstances, we would likely want to use API graphs to do this, however because we're working on a layer that lives below the API graph computation we have to use a more syntactic approximation. (It's probably fine, though. If you decorate something with overload in some way, well, you're probably doing some overloading.)

Still, I should add a comment about this, and also make the method private.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in f2bad1e.

|
func.getADecorator() = overload.getNode()
)
}

// =============================================================================
// Callables
// =============================================================================
Expand Down Expand Up @@ -849,7 +868,8 @@ private Class getNextClassInMro(Class cls) {
*/
Function findFunctionAccordingToMro(Class cls, string name) {
result = cls.getAMethod() and
result.getName() = name
result.getName() = name and
not hasOverloadDecorator(result)
or
not class_has_method(cls, name) and
result = findFunctionAccordingToMro(getNextClassInMro(cls), name)
Expand Down Expand Up @@ -891,6 +911,7 @@ Class getNextClassInMroKnownStartingClass(Class cls, Class startingClass) {
Function findFunctionAccordingToMroKnownStartingClass(Class cls, Class startingClass, string name) {
result = cls.getAMethod() and
result.getName() = name and
not hasOverloadDecorator(result) and
cls = getADirectSuperclass*(startingClass)
or
not class_has_method(cls, name) and
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/**
* Test that `@typing.overload` stubs are not resolved as call targets.
*/

import python
import semmle.python.dataflow.new.internal.DataFlowDispatch as DataFlowDispatch
import utils.test.InlineExpectationsTest

module OverloadCallTest implements TestSig {
string getARelevantTag() { result = "init" }

predicate hasActualResult(Location location, string element, string tag, string value) {
exists(location.getFile().getRelativePath()) and
exists(DataFlowDispatch::DataFlowCall call, Function target |
location = call.getLocation() and
element = call.toString() and
DataFlowDispatch::resolveCall(call.getNode(), target, _) and
target.getName() = "__init__"
|
value = target.getQualifiedName() + ":" + target.getLocation().getStartLine().toString() and
tag = "init"
)
}
}

import MakeTest<OverloadCallTest>
39 changes: 39 additions & 0 deletions python/ql/test/library-tests/dataflow/calls-overload/test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import typing


class OverloadedInit:
@typing.overload
def __init__(self, x: int) -> None: ...

@typing.overload
def __init__(self, x: str, y: str) -> None: ...

def __init__(self, x, y=None):
pass

OverloadedInit(1) # $ init=OverloadedInit.__init__:11
OverloadedInit("a", "b") # $ init=OverloadedInit.__init__:11


from typing import overload


class OverloadedInitFromImport:
@overload
def __init__(self, x: int) -> None: ...

@overload
def __init__(self, x: str, y: str) -> None: ...

def __init__(self, x, y=None):
pass

OverloadedInitFromImport(1) # $ init=OverloadedInitFromImport.__init__:28
OverloadedInitFromImport("a", "b") # $ init=OverloadedInitFromImport.__init__:28


class NoOverloads:
def __init__(self, x):
pass

NoOverloads(1) # $ init=NoOverloads.__init__:36