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

SONARPY-2188 Introduce UnresolvedImportedType #2056

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ghislainpiot
Copy link
Contributor

@ghislainpiot ghislainpiot commented Oct 10, 2024

This pull request introduces a new UnresolvedImportType to handle unknown imports in Python files and updates the type inference logic accordingly. Additionally, it includes several test updates to verify the new behavior.

Type Inference Enhancements:

  • Added UnresolvedImportType class to handle unresolved imports. (python-frontend/src/main/java/org/sonar/python/types/v2/UnresolvedImportType.java)
  • Updated TrivialTypeInferenceVisitor to set UnresolvedImportType for unknown imports. (python-frontend/src/main/java/org/sonar/python/semantic/v2/types/TrivialTypeInferenceVisitor.java) [1] [2] [3] [4]

Test Updates:

  • Modified tests to assert the new UnresolvedImportType behavior. (python-frontend/src/test/java/org/sonar/python/semantic/v2/TypeInferenceV2Test.java) [1] [2] [3]
  • Replaced Assertions.assertThat with assertThat for consistency. (python-frontend/src/test/java/org/sonar/python/semantic/v2/TypeInferenceV2Test.java) [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15] [16] [17] [18] [19]

.orElse(null);
}
}
} else if (resolvedType instanceof UnknownType || (resolvedType instanceof ObjectType objectType && objectType.type() instanceof UnknownType)) {

Choose a reason for hiding this comment

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

Do we get ObjectType[UnknownType] somewhere? Is it expected? We probably want to avoid it (should we create a ticket?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We do get them.
They are probably coming from the converters and the systematic wrapping in ObjectType.
I think right now, it is unexpected and is happening for the wrong reasons.
I will create a ticket

In the future, I think we might have some. As long as we know for sure it is an object.

}
} else if (resolvedType instanceof UnknownType || (resolvedType instanceof ObjectType objectType && objectType.type() instanceof UnknownType)) {
if (aliasedName.alias() != null) {
var unresolvedImportType = new UnresolvedImportType(String.join(".", fqn));

Choose a reason for hiding this comment

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

For this prototype/investigation, it doesn't matter, but when we actually implement this we should probably try to ensure we don't create duplicate types for the same FQN if we see stuff like:

def foo():
    from lib import some_func

def bar():
    from lib import some_func 

Copy link

Quality Gate failed Quality Gate failed

Failed conditions
1 New issue
89.7% Coverage on New Code (required ≥ 95%)

See analysis details on SonarQube

Catch issues before they fail your Quality Gate with our IDE extension SonarLint SonarLint

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants