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-1826 Enable flow sensitive type inference for function types #1796

Merged
merged 2 commits into from
May 14, 2024

Conversation

guillaume-dequenne-sonarsource
Copy link
Contributor

No description provided.

public abstract class Propagation {

final Set<SymbolV2> variableDependencies = new HashSet<>();
final Set<Propagation> dependents = new HashSet<>();

Choose a reason for hiding this comment

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

initialize in the constructor?

Comment on lines 53 to 60
lhsSymbol.usages().stream()
.filter(u -> !SymbolV2Utils.isFunctionOrClassDeclaration(u))
.map(UsageV2::tree)
.filter(NameImpl.class::isInstance)
.map(NameImpl.class::cast)
.forEach(n -> n.typeV2(rhsType));

Choose a reason for hiding this comment

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

this part is the same as usages stream below, so maybe it makes sense to extend this common part?

Comment on lines +90 to +101
return lhsName.symbolV2().usages()
.stream()
.filter(u -> !SymbolV2Utils.isFunctionOrClassDeclaration(u))
.map(UsageV2::tree)

Choose a reason for hiding this comment

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

maybe we can simply move this part to SymbolV2Utils:

public static Stream<Tree> getSymbolNonDeclarationUsageTrees(SymbolV2 symbol) {
  
  return symbol.usages()
      .stream()
      .filter(u -> !SymbolV2Utils.isFunctionOrClassDeclaration(u))
      .map(UsageV2::tree)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point! I extracted this into a dedicated method.
However, I kept it in Propagation, as I want the filtering out of definitions to be super explicit (I think it only makes sense in this specific case). I added a comment to clarify this in the now mutualized method.

Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM, see a couple of small comments

Base automatically changed from SONARPY-1825 to MMF-3796 May 13, 2024 14:41
@guillaume-dequenne-sonarsource guillaume-dequenne-sonarsource changed the title SONARPY-1826 tmp SONARPY-1826 Enable flow sensitive type inference for function types May 13, 2024
Copy link

Quality Gate failed Quality Gate failed

Failed conditions
3 New issues

See analysis details on SonarQube

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

@guillaume-dequenne-sonarsource guillaume-dequenne-sonarsource marked this pull request as ready for review May 14, 2024 07:13
@guillaume-dequenne-sonarsource guillaume-dequenne-sonarsource merged commit 4920279 into MMF-3796 May 14, 2024
0 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants