Repository navigation
SONARJAVA-7288 Replace Eclipse JDT core with PSI in the Java frontend - #6456
aurelien-coet-sonarsource wants to merge 2 commits into
Conversation
hashicorp-vault-sonar-prod
Bot
changed the title
|
/pvf |
6926b11 to
8c6424f
Compare
CI failed: No actual CI failure is identifiable in the supplied logs; the excerpts show only successful setup, cache misses (non-fatal), diff report generation and upload, and cleanup, so the cause is unclear and not tied to the PSI change.OverviewTwo unique log excerpts were analyzed across 7 logs and 7 error templates, but neither shows an explicit error, failed step, nonzero exit, or job conclusion. The visible steps succeeded and the cache and StepSecurity messages are warnings. The failing check cannot be identified from the supplied content. FailuresNo identifiable failure in Ruling QA logs (confidence: low)
Summary
Code Review
|
| Auto-apply | Compact | Unblock |
|
|
|
Was this helpful? React with 👍 / 👎 | Gitar
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
|
|
||
| @Override | ||
| public Runnable getEnvironmentCleaner() { | ||
| if (environmentCleaner == null) { | ||
| environmentCleaner = FrontendResources.idempotent(ASTUtils.getEnvironmentCleaner(ast)); | ||
| String binaryName(PsiClass cls) { | ||
| if (cls.getContainingClass() != null) { | ||
| return binaryName(cls.getContainingClass()) + "$" + cls.getName(); | ||
| } | ||
| return environmentCleaner; | ||
| if (cls.getQualifiedName() != null) { | ||
| return cls.getQualifiedName(); | ||
| } | ||
| var outer = PsiTreeUtil.getParentOfType(cls, PsiClass.class, true); | ||
| if (outer == null) { | ||
| return cls.getName() == null ? "" : cls.getName(); | ||
| } | ||
| int ordinal = 0; | ||
| for (var candidate : PsiTreeUtil.findChildrenOfType(outer, PsiClass.class)) { | ||
| if (candidate instanceof PsiAnonymousClass || candidate.getQualifiedName() == null) { |
There was a problem hiding this comment.
💡 Bug: binaryName gives wrong numbers to anonymous and local classes
JSema.binaryName numbers a class with no qualified name by counting every descendant PsiClass of the outer class that has no qualified name (PsiTreeUtil.findChildrenOfType(outer, PsiClass.class)). This walk is depth-first and recursive, and it puts anonymous and local classes on one counter. Two cases come out wrong:
- In
class A { Object a = new Object(){ Object b = new Object(){}; }; Object c = new Object(){}; }, the second top-level anonymous class becomesA$3, because the class nested inside the first one is counted too. javac/ECJ name itA$2. - In
void m(){ new Object(){}; class L{} }, the local class becomesA$2Linstead ofA$1L.
The result reaches Type.fullyQualifiedName() and MethodSymbol.signature(), so checks that match on binary names see values that differ from the old ECJ getBinaryName().
Fix: count only direct nested classes, skipping classes inside other anonymous or local classes. Keep a separate counter for each local class name.
Was this helpful? React with 👍 / 👎
| if (type instanceof PsiClassType cls) { | ||
| var resolved = cls.resolve(); | ||
| if (resolved instanceof PsiTypeParameter parameter) { | ||
| return parameter.getName(); | ||
| } | ||
| if (resolved != null) { | ||
| return sema.binaryName(resolved); | ||
| } | ||
| String name = cls.rawType().getCanonicalText(false); | ||
| return (cls instanceof PsiClassReferenceType reference && reference.getReference().getParent() instanceof PsiReferenceList) | ||
| || sema.explicitRuntime || sema.getClassType("java.lang.Object").isUnknown() ? "Recovered#typeBindingL" + name.replace('.', '/') + ";0" : name; | ||
| } |
There was a problem hiding this comment.
💡 Edge Case: Unresolved type's FQN depends on which usage is converted first
JType.qualifiedName gives an unresolved class type one of two forms. If the reference sits in a PsiReferenceList (extends/implements), it returns Recovered#typeBindingL…;0; otherwise it returns the plain canonical name. But JSema.typeKey keys every unresolved class type only by "recovered:" + canonicalText, and JSema.type caches one JType per key. So in class A extends Missing { Missing f; }, the field type and the supertype share a single JType. Whichever PsiType is converted first decides the fullyQualifiedName() that both report. The behavior the branch is meant to distinguish therefore depends on traversal order.
Fix: include the reference-list context in the key, or compute the recovered name from the type alone so it does not depend on context.
Was this helpful? React with 👍 / 👎
❌ ErrorsYour PR has failed checks. Please review the issues below and take necessary action before merging. 🚦 8 Pipeline jobs failed
Useful? React with 👍 / 👎 This comment will be updated automatically if new data arrives.🔗 Commit SHA: 8c6424f | Docs | View more details | Give us feedback! |
|
📊 Performance Validation complete Projects with unexpected PVF results:
|
|
⚖️ Ruling update ready for review: #6458 |
Summary by Gitar
JParser,JSema,JType, andJSymbolnow resolve types and symbols through PSI instead of ECJ bindingsPsiFrontendBackendandPsiSemanticAttachmentas the new semantic backendEcj*model classes andorg.eclipse.jdt.core.dom.ASTUtils, and dropped thejava-frontendpom dependencyFrontendDependencyValidatorto fail packaging if ECJ or JDT classes appear in the plugin JARSemanticTestSupportand added PSI-specific suites such asPsiFrontendBackendTestandJSemaRegressionTestThis will update automatically on new commits.