From b088c1d7292959018111341b47314aa765da9b8d Mon Sep 17 00:00:00 2001 From: Gayan Perera Date: Thu, 11 Apr 2024 19:17:08 +0200 Subject: [PATCH] Record pattern variable resolution inside switch, Fixes #2299 + approach by manual recovery of a switch statement + minimal fixes for elementStack handling of record patterns --- .../CompletionTestsForRecordPattern.java | 57 ++++++++++++++- .../complete/CompletionNodeDetector.java | 20 ++++++ .../codeassist/complete/CompletionParser.java | 70 ++++++++++++++++++- .../codeassist/impl/AssistParser.java | 3 + 4 files changed, 148 insertions(+), 2 deletions(-) diff --git a/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/CompletionTestsForRecordPattern.java b/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/CompletionTestsForRecordPattern.java index 8aa346ec010..80d809293e0 100644 --- a/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/CompletionTestsForRecordPattern.java +++ b/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/CompletionTestsForRecordPattern.java @@ -19,10 +19,12 @@ import junit.framework.Test; public class CompletionTestsForRecordPattern extends AbstractJavaModelCompletionTests { + private static int UNQUALIFIED_REL = R_DEFAULT + R_RESOLVED + R_CASE + R_INTERESTING + R_UNQUALIFIED + + R_NON_RESTRICTED; static { -// TESTS_NAMES = new String[]{"test012"}; + //TESTS_NAMES = new String[]{"testGH2299_SwitchStatement"}; } public CompletionTestsForRecordPattern(String name) { @@ -558,4 +560,57 @@ public void test015() throws JavaModelException { assertResults("ar_ray[LOCAL_VARIABLE_REF]{ar_ray, null, [LColoredRectangle;, ar_ray, null, 52}", requestor.getResults()); } + + public void testGH2299_SwitchStatement() throws JavaModelException { + this.workingCopies = new ICompilationUnit[2]; + this.workingCopies[0] = getWorkingCopy("/Completion/src/SwitchRecordPattern.java", """ + public class SwitchRecordPattern { + public void foo(java.io.Serializable o) { + switch(o) { + case Person(var name, var age) : { + /*here*/nam + } + } + } + }\ + """); + this.workingCopies[1] = getWorkingCopy("/Completion/src/Person.java", """ + public record Person(String name, int age) implements java.io.Serializable {}\ + """); + CompletionTestsRequestor2 requestor = new CompletionTestsRequestor2(true); + requestor.allowAllRequiredProposals(); + String str = this.workingCopies[0].getSource(); + String completeBehind = "/*here*/nam"; + int cursorLocation = str.lastIndexOf(completeBehind) + completeBehind.length(); + this.workingCopies[0].codeComplete(cursorLocation, requestor, this.wcOwner); + assertResults("name[LOCAL_VARIABLE_REF]{name, null, Ljava.lang.String;, name, null, " + UNQUALIFIED_REL + "}", + requestor.getResults()); + } + + public void testGH2299_SwitchExpression() throws JavaModelException { + this.workingCopies = new ICompilationUnit[2]; + this.workingCopies[0] = getWorkingCopy("/Completion/src/SwitchRecordPattern.java", """ + public class SwitchRecordPattern { + public void foo(java.io.Serializable o) { + String result = switch(o) { + case Person(var name, var age) -> { + /*here*/nam + } + }; + } + }\ + """); + this.workingCopies[1] = getWorkingCopy("/Completion/src/Person.java", """ + public record Person(String name, int age) implements java.io.Serializable {}\ + """); + CompletionTestsRequestor2 requestor = new CompletionTestsRequestor2(true); + requestor.allowAllRequiredProposals(); + String str = this.workingCopies[0].getSource(); + String completeBehind = "/*here*/nam"; + int cursorLocation = str.lastIndexOf(completeBehind) + completeBehind.length(); + this.workingCopies[0].codeComplete(cursorLocation, requestor, this.wcOwner); + assertResults("name[LOCAL_VARIABLE_REF]{name, null, Ljava.lang.String;, name, null, " + UNQUALIFIED_REL + "}", + requestor.getResults()); + } + } diff --git a/org.eclipse.jdt.core/codeassist/org/eclipse/jdt/internal/codeassist/complete/CompletionNodeDetector.java b/org.eclipse.jdt.core/codeassist/org/eclipse/jdt/internal/codeassist/complete/CompletionNodeDetector.java index 1841b12e7cb..6efc7959da7 100644 --- a/org.eclipse.jdt.core/codeassist/org/eclipse/jdt/internal/codeassist/complete/CompletionNodeDetector.java +++ b/org.eclipse.jdt.core/codeassist/org/eclipse/jdt/internal/codeassist/complete/CompletionNodeDetector.java @@ -276,10 +276,16 @@ public void endVisit(SuperReference superReference, BlockScope scope) { @Override public void endVisit(SwitchStatement switchStatement, BlockScope scope) { endVisit(switchStatement); + if (this.parent == switchStatement && !isOnCompletingOnCaseLabel(switchStatement)) { + this.parent = NOT_A_PARENT; + } } @Override public void endVisit(SwitchExpression switchExpression, BlockScope scope) { endVisit(switchExpression); + if (this.parent == switchExpression && !isOnCompletingOnCaseLabel(switchExpression)) { + this.parent = NOT_A_PARENT; + } } @Override public void endVisit(ThisReference thisReference, BlockScope scope) { @@ -530,6 +536,20 @@ private void endVisit(ASTNode astNode) { } } + private boolean isOnCompletingOnCaseLabel(SwitchStatement statement) { + for (Statement stmt : statement.statements) { + if (stmt instanceof CaseStatement cs) { + for (Expression expr : cs.constantExpressions) { + if (this.searchedNode == expr + || (expr instanceof RecordPattern rp && rp.type == this.searchedNode)) { + return true; + } + } + } + } + return false; + } + protected void checkUpdateOuter(ASTNode astNode) { if (this.containsPotentialPolyExpression && astNode instanceof Expression) { // resolving a contained poly expression can only benefit from any additional expression context diff --git a/org.eclipse.jdt.core/codeassist/org/eclipse/jdt/internal/codeassist/complete/CompletionParser.java b/org.eclipse.jdt.core/codeassist/org/eclipse/jdt/internal/codeassist/complete/CompletionParser.java index c6da74a7421..5c84eddaa37 100644 --- a/org.eclipse.jdt.core/codeassist/org/eclipse/jdt/internal/codeassist/complete/CompletionParser.java +++ b/org.eclipse.jdt.core/codeassist/org/eclipse/jdt/internal/codeassist/complete/CompletionParser.java @@ -126,6 +126,7 @@ import org.eclipse.jdt.internal.compiler.parser.RecoveredModule; import org.eclipse.jdt.internal.compiler.parser.RecoveredPackageVisibilityStatement; import org.eclipse.jdt.internal.compiler.parser.RecoveredProvidesStatement; +import org.eclipse.jdt.internal.compiler.parser.RecoveredStatement; import org.eclipse.jdt.internal.compiler.parser.RecoveredType; import org.eclipse.jdt.internal.compiler.parser.RecoveredUnit; import org.eclipse.jdt.internal.compiler.parser.Scanner; @@ -1192,6 +1193,41 @@ private void buildMoreCompletionContext(Expression expression) { this.assistNodeParent = switchStatement; } break; + case K_SWITCH_EXPRESSION_DELIMITTER: // at "case -> {" ? + case K_BLOCK_DELIMITER: // at "case : {" ? + if (lastIndexOfElement(K_SWITCH_LABEL) > -1 + && this.expressionPtr > 0 + && this.astLengthPtr > -1 + && this.astPtr > -1 + && this.currentElement instanceof RecoveredBlock) + { + int length = this.astLengthStack[this.astLengthPtr]; + int newAstPtr = this.astPtr - length; + ASTNode firstNode = this.astStack[newAstPtr + 1]; + if (popBlockContaining(firstNode)) { + // established: the parts of the switch had been captured in recovered elements. + // now we replace those parts with a manually assembled switch statement: + SwitchStatement switchStatement = new SwitchStatement(); + switchStatement.expression = this.expressionStack[this.expressionPtr - 1]; + if(length != 0 && firstNode.sourceStart > switchStatement.expression.sourceEnd) { + // transfer existing case statements: + switchStatement.statements = new Statement[length + 1]; + System.arraycopy( + this.astStack, + newAstPtr + 1, + switchStatement.statements, + 0, + length); + } + // now attach the orphan expression: + Block block = new Block(0); + block.statements = new Statement[] { expression }; + switchStatement.statements[switchStatement.statements.length-1] = block; + this.currentElement.add(switchStatement, 0); + break nextElement; + } + } + break; case K_BETWEEN_IF_AND_RIGHT_PAREN : IfStatement ifStatement = new IfStatement(expression, new EmptyStatement(expression.sourceEnd, expression.sourceEnd), expression.sourceStart, expression.sourceEnd); this.assistNodeParent = ifStatement; @@ -1255,6 +1291,28 @@ private void buildMoreCompletionContext(Expression expression) { } } } +private boolean popBlockContaining(ASTNode soughtStatement) { + // check if soughtStatement was prematurely captured in a RecoveredStatement up the parent chain. + // if so, pop until the next parent. + RecoveredElement elem = this.currentElement; + while (elem instanceof RecoveredBlock block) { + for (int i=0; i 0) { @@ -4138,6 +4196,11 @@ protected void consumeToken(int token) { if(previous == TokenNameIdentifier && topKnownElementKind(COMPLETION_OR_ASSIST_PARSER) == K_PARAMETERIZED_METHOD_INVOCATION) { popElement(K_PARAMETERIZED_METHOD_INVOCATION); + } else if (previous == TokenNameIdentifier + && topKnownElementKind(COMPLETION_OR_ASSIST_PARSER) == K_BETWEEN_CASE_AND_COLON) { + // replace, ID( is no longer a regular case constant, but starts a record pattern: + popElement(K_BETWEEN_CASE_AND_COLON); + pushOnElementStack(K_RECORD_PATTERN); } else { popElement(K_BETWEEN_NEW_AND_LEFT_BRACKET); } @@ -4176,6 +4239,11 @@ protected void consumeToken(int token) { popElement(K_BETWEEN_LEFT_AND_RIGHT_BRACKET); } break; + case TokenNameARROW, TokenNameCOLON: + if(topKnownElementKind(COMPLETION_OR_ASSIST_PARSER) == K_RECORD_PATTERN) { + popElement(K_RECORD_PATTERN); + } + break; } } diff --git a/org.eclipse.jdt.core/codeassist/org/eclipse/jdt/internal/codeassist/impl/AssistParser.java b/org.eclipse.jdt.core/codeassist/org/eclipse/jdt/internal/codeassist/impl/AssistParser.java index 35a79de3483..194c44f7722 100644 --- a/org.eclipse.jdt.core/codeassist/org/eclipse/jdt/internal/codeassist/impl/AssistParser.java +++ b/org.eclipse.jdt.core/codeassist/org/eclipse/jdt/internal/codeassist/impl/AssistParser.java @@ -113,6 +113,7 @@ public abstract class AssistParser extends Parser { protected static final int K_LAMBDA_EXPRESSION_DELIMITER = ASSIST_PARSER + 7; // whether we are inside a lambda expression protected static final int K_MODULE_INFO_DELIMITER = ASSIST_PARSER + 8; // whether we are inside a module info declaration protected static final int K_SWITCH_EXPRESSION_DELIMITTER = ASSIST_PARSER + 9; // whether we are inside a switch expression + protected static final int K_RECORD_PATTERN = ASSIST_PARSER + 10; // whether we are inside record pattern // selector constants protected static final int THIS_CONSTRUCTOR = -1; @@ -1366,6 +1367,8 @@ protected void consumeToken(int token) { adjustBracket(token); switch (token) { case TokenNameLPAREN : + if (topKnownElementKind(ASSIST_PARSER) == K_RECORD_PATTERN) + break; // after 'case' 'ID(' is *not* the start of an invocation switch (this.previousToken) { case TokenNameIdentifier: this.pushOnElementStack(K_SELECTOR, this.identifierPtr);