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-2219 Pass the result of the type inference for a Python file to ProjectLevelTypesTable #2090

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

Conversation

thomas-serre-sonarsource
Copy link
Contributor

No description provided.

assertThat(ambiguousDescriptor.name()).isEqualTo("myUnionType");
assertThat(ambiguousDescriptor.fullyQualifiedName()).isEqualTo("foo.myUnionType");
assertThat(ambiguousDescriptor.kind()).isEqualTo(Descriptor.Kind.AMBIGUOUS);
// TODO SONARPY-2225 the two class types in the union are rigorously the same but the converter creates an ambigouous descriptor

Choose a reason for hiding this comment

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

I believe it is to be expected since we don't implement equals for PythonType and expect the unicity of PythonType objects for each available type.

I guess that means that we'd have an ambiguous descriptor in the case where there are 2 definitions that are strictly identical:

if x:
  class A: ...
else:
  class A: ...

However, I think it's okay to accept this behavior for now. If this prevents proper resolution in some cases, we can always implement something that would merge types that are essentially identical.

@maksim-grebeniuk-sonarsource maksim-grebeniuk-sonarsource force-pushed the ts/SONARPY-2111/SONARPY-2219 branch 3 times, most recently from 65a0157 to 6115ada Compare October 25, 2024 14:14
Copy link

Quality Gate failed Quality Gate failed

Failed conditions
14 New issues

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.

3 participants