Skip to content

Commit

Permalink
Record pattern variable resolution inside switch, Fixes eclipse-jdt#2299
Browse files Browse the repository at this point in the history 


+ approach by manual recovery of a switch statement
+ minimal fixes for elementStack handling of record patterns
  • Loading branch information
@gayanper @stephan-herrmann
gayanper authored and stephan-herrmann committed Apr 26, 2024
1 parent af7e974 commit b088c1d
Show file tree
Hide file tree
Showing 4 changed files with 148 additions and 2 deletions.
57 changes: 56 additions & 1 deletion ...ore.tests.model/src/org/eclipse/jdt/core/tests/model/CompletionTestsForRecordPattern.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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());
}

}
20 changes: 20 additions & 0 deletions ....core/codeassist/org/eclipse/jdt/internal/codeassist/complete/CompletionNodeDetector.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down
70 changes: 69 additions & 1 deletion ...se.jdt.core/codeassist/org/eclipse/jdt/internal/codeassist/complete/CompletionParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -1192,6 +1193,41 @@ private void buildMoreCompletionContext(Expression expression) {
this.assistNodeParent = switchStatement;
}
break;
case K_SWITCH_EXPRESSION_DELIMITTER: // at "case <something> -> {" ?
case K_BLOCK_DELIMITER: // at "case <something> : {" ?
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;
Expand Down Expand Up @@ -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<block.statementCount; i++) {
if (block.statements[i] instanceof RecoveredStatement stmt) {
if (stmt.statement == soughtStatement) {
this.currentElement = block.parent;
// also remove block from the new currentElement:
if (this.currentElement instanceof RecoveredBlock newBlock) {
if (newBlock.statements[newBlock.statementCount-1] == block)
newBlock.statementCount--;
}
return true;
}
}
}
elem = elem.parent;
}
return false;
}
private Statement buildMoreCompletionEnclosingContext(Statement statement) {
IfStatement ifStatement = null;
int index = -1;
Expand Down Expand Up @@ -2295,7 +2353,7 @@ private void classHeaderExtendsOrImplements(boolean isInterface, boolean isRecor
if (sealed)
keywords[count++] = RestrictedIdentifiers.PERMITS;
}

System.arraycopy(keywords, 0, keywords = new char[count][], 0, count);

if (count > 0) {
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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;

}
}
Expand Down
3 changes: 3 additions & 0 deletions org.eclipse.jdt.core/codeassist/org/eclipse/jdt/internal/codeassist/impl/AssistParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit b088c1d

Please sign in to comment.