Skip to content

Commit

Permalink
Scanner's recognition of restricted identifier "when" is fragile (#1436)
Browse files Browse the repository at this point in the history
Consult the parser's state to disambiguate identifier "when" from restricted key word "when"

* Fixes #609
* Fixes #456
* Fixes eclipse-jdt/eclipse.jdt#59
  • Loading branch information
srikanth-sankaran authored Oct 2, 2023
1 parent a0106eb commit 983c3bf
Show file tree
Hide file tree
Showing 9 changed files with 312 additions and 109 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,13 @@ public interface ConflictedParser {
we treat the type annotation as a declarative annotation.
*/
boolean atConflictScenario(int token);

/* Return true if at the configuration the parser finds itself in, it would shift the token.
It is axiomatic of the push down automaton that corresponds to the LALR grammar that it
will never shift on invalid input.
*/
boolean automatonWillShift(int token);

/*
* Return true if the parser is parsing a module declaration. In Java 9, module, requires, exports,
* to, uses, provides, and with are restricted keywords (i.e. they are keywords solely where they
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14768,6 +14768,12 @@ public boolean automatonWillShift(int token, int lastAction) {
return lastAction != ERROR_ACTION;
}
}

@Override
public boolean automatonWillShift(int token) {
return automatonWillShift(token, this.unstackedAct);
}

@Override
public boolean isParsingJava14() {
return this.parsingJava14Plus;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2000, 2022 IBM Corporation and others.
* Copyright (c) 2000, 2023 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
Expand Down Expand Up @@ -4780,10 +4780,8 @@ private static class Goal {
static int BlockStatementoptRule = 0;
static int YieldStatementRule = 0;
static int SwitchLabelCaseLhsRule = 0;
static int GuardRule = 0;
static int[] RestrictedIdentifierSealedRule;
static int[] RestrictedIdentifierPermitsRule;
static int[] RestrictedIdentifierWhenRule;
static int[] PatternRules;
static int RecordPatternRule = 0;

Expand All @@ -4796,16 +4794,13 @@ private static class Goal {
static Goal SwitchLabelCaseLhsGoal;
static Goal RestrictedIdentifierSealedGoal;
static Goal RestrictedIdentifierPermitsGoal;
static Goal RestrictedIdentifierWhenGoal;
static Goal PatternGoal;
static Goal RecordPatternGoal;

static int[] EMPTY_FOLLOW_SET = new int[0];
static int[] RestrictedIdentifierSealedFollow = { TokenNameclass, TokenNameinterface,
TokenNameenum, TokenNameRestrictedIdentifierrecord };// Note: enum/record allowed as error flagging rules.
static int[] RestrictedIdentifierPermitsFollow = { TokenNameLBRACE };
static int[] PatternCaseLabelFollow = {TokenNameCOLON, TokenNameARROW, TokenNameCOMMA, TokenNameBeginCaseExpr, TokenNameRestrictedIdentifierWhen};
static int[] GuardFollow = EMPTY_FOLLOW_SET;
static int[] RecordPatternFollow = {TokenNameCOLON}; // disambiguate only for enh for

static {
Expand Down Expand Up @@ -4854,10 +4849,7 @@ private static class Goal {
if ("RecordPattern".equals(Parser.name[Parser.non_terminal_index[Parser.lhs[i]]])) {//$NON-NLS-1$
patternStates.add(i);
RecordPatternRule = i;
} else
if ("Expression".equals(Parser.name[Parser.non_terminal_index[Parser.lhs[i]]])) //$NON-NLS-1$
GuardRule = i;

}
}
RestrictedIdentifierSealedRule = ridSealed.stream().mapToInt(Integer :: intValue).toArray(); // overkill but future-proof
RestrictedIdentifierPermitsRule = ridPermits.stream().mapToInt(Integer :: intValue).toArray();
Expand All @@ -4872,7 +4864,6 @@ private static class Goal {
SwitchLabelCaseLhsGoal = new Goal(TokenNameARROW, new int [0], SwitchLabelCaseLhsRule);
RestrictedIdentifierSealedGoal = new Goal(TokenNameRestrictedIdentifiersealed, RestrictedIdentifierSealedFollow, RestrictedIdentifierSealedRule);
RestrictedIdentifierPermitsGoal = new Goal(TokenNameRestrictedIdentifierpermits, RestrictedIdentifierPermitsFollow, RestrictedIdentifierPermitsRule);
RestrictedIdentifierWhenGoal = new Goal(TokenNameRestrictedIdentifierWhen, GuardFollow, GuardRule);
PatternGoal = new Goal(TokenNameBeginCaseElement, PatternCaseLabelFollow, PatternRules);
RecordPatternGoal = new Goal(TokenNameQUESTION, RecordPatternFollow, RecordPatternRule);
}
Expand Down Expand Up @@ -4985,6 +4976,11 @@ protected boolean parse(Goal goal) {
do { /* reduce */
if (goal.hasBeenReached(act, this.currentToken))
return SUCCESS;
if (this.currentToken == TokenNameIdentifier) {
int reskw = TerminalTokens.getRestrictedKeyword(this.scanner.getCurrentIdentifierSource());
if (reskw != TokenNameNotAToken && goal.hasBeenReached(act, reskw))
return SUCCESS;
}
this.stateStackTop -= (Parser.rhs[act] - 1);
act = Parser.ntAction(this.stack[this.stateStackTop], Parser.lhs[act]);
} while (act <= NUM_RULES);
Expand Down Expand Up @@ -5080,25 +5076,6 @@ protected final boolean mayBeAtCasePattern(int token) {
return (!isInModuleDeclaration() && JavaFeature.PATTERN_MATCHING_IN_SWITCH.isSupported(this.complianceLevel, this.previewEnabled))
&& (token == TokenNamecase || this.multiCaseLabelComma);
}
protected boolean mayBeAtGuard(int token) {
if (isInModuleDeclaration())
return false;
if (!JavaFeature.PATTERN_MATCHING_IN_SWITCH.isSupported(this.complianceLevel, this.previewEnabled))
return false;
/*
* A simple elimination optimization for some common possible cases. According to the JLS 19 including
* patterns-switch and record-patterns Section 14.30.1, a guard may only be preceded by either right parentheses or
* an identifier. However, we may still encounter comments, whitespace or the not-a-token token.
*/
switch (this.lookBack[1]) {
case TokenNameRPAREN:
case TokenNameIdentifier:
case TokenNameNotAToken: // TODO is this useful? Some tests start scanning at "when", but this makes no sense as a Pattern is required by the JLS
return true;
}
return false;
}

protected final boolean maybeAtLambdaOrCast() { // Could the '(' we saw just now herald a lambda parameter list or a cast expression ? (the possible locations for both are identical.)

if (isInModuleDeclaration())
Expand All @@ -5112,6 +5089,7 @@ protected final boolean maybeAtLambdaOrCast() { // Could the '(' we saw just now
case TokenNameswitch:
case TokenNamewhile:
case TokenNamefor:
case TokenNamecase:
case TokenNamesynchronized:
case TokenNametry:
return false; // not a viable prefix for cast or lambda.
Expand Down Expand Up @@ -5407,8 +5385,8 @@ int disambiguatedRestrictedIdentifierWhen(int restrictedIdentifierToken) {
if (!JavaFeature.PATTERN_MATCHING_IN_SWITCH.isSupported(this.complianceLevel, this.previewEnabled))
return TokenNameIdentifier;

return disambiguatesRestrictedIdentifierWithLookAhead(this::mayBeAtGuard,
restrictedIdentifierToken, Goal.RestrictedIdentifierWhenGoal);
return this.activeParser == null || !this.activeParser.automatonWillShift(TokenNameRestrictedIdentifierWhen) ?
TokenNameIdentifier : TokenNameRestrictedIdentifierWhen;
}
int disambiguatedRestrictedIdentifierYield(int restrictedIdentifierToken) {
// and here's the kludge
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,38 @@ public interface TerminalTokens {
TokenNameCOMMENT_JAVADOC = 1003,
TokenNameSingleQuoteStringLiteral = 1004;

static boolean isRestrictedKeyword(int tokenType) {
return switch (tokenType) {
case TokenNameRestrictedIdentifierYield, TokenNameRestrictedIdentifierrecord,TokenNameRestrictedIdentifierWhen,
TokenNameRestrictedIdentifiersealed, TokenNameRestrictedIdentifierpermits -> true;
default -> false;
};
}

static int getRestrictedKeyword(char [] text) {
if (text != null) {
int len = text.length;
if (len == 4 && text[0] == 'w' ||
len == 5 && text[0] == 'y' ||
len == 6 && (text[0] == 'r' || text[0] == 's') ||
len == 7 && text[0] == 'p') {
return getRestrictedKeyword(new String(text));
}
}
return TokenNameNotAToken;
}

static int getRestrictedKeyword(String text) {
return switch (text) {
case "yield" -> TokenNameRestrictedIdentifierYield; //$NON-NLS-1$
case "record" -> TokenNameRestrictedIdentifierrecord; //$NON-NLS-1$
case "when" -> TokenNameRestrictedIdentifierWhen; //$NON-NLS-1$
case "sealed" -> TokenNameRestrictedIdentifiersealed; //$NON-NLS-1$
case "permits" -> TokenNameRestrictedIdentifierpermits; //$NON-NLS-1$
default -> TokenNameNotAToken;
};
}

// BEGIN_AUTOGENERATED_REGION
int TokenNameIdentifier = 19,
TokenNameabstract = 42,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2611,6 +2611,11 @@ public boolean atConflictScenario(int token) {
return (token == TokenNameLPAREN || token == TokenNameAT || (token == TokenNameLESS && !this.lexStream.awaitingColonColon()));
}

@Override
public boolean automatonWillShift(int token) {
return false; // Some day we will understand the world well enough, for now say no and deal with it (sigh)
}

@Override
public boolean isParsingModuleDeclaration() {
//
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1790,28 +1790,6 @@ public void testModule() { // insufficient context, all module words are identif
assertTrue(false);
}
}
public void testWhenOK() {
String source = ("public void foo(Object obj) {\n switch(obj) {\n case String s when s.length() > 0 -> {}\n}\n}");
IScanner scanner = ToolFactory.createScanner(false, true, false, "21", "21", false);
scanner.setSource(source.toCharArray());
scanner.resetTo(source.indexOf("when")-1, source.length() - 1); // start directly at "when"
try {
int token;
while ((token = scanner.getNextToken()) != ITerminalSymbols.TokenNameEOF) {
switch (token) {
case ITerminalSymbols.TokenNameWHITESPACE:
break;
case ITerminalSymbols.TokenNameRestrictedIdentifierWhen:
return; // success
default:
fail("Unexpected token "+token);
}
}
fail("TokenNameRestrictedIdentifierYield was not detected");
} catch (InvalidInputException e) {
assertTrue(false);
}
}
@SuppressWarnings("deprecation")
public void testWhenKO() {
String source = ("public void foo(Object obj) {\n switch(obj) {\n case String s when s.length() > 0 -> {}\n}\n}");
Expand Down Expand Up @@ -1857,4 +1835,24 @@ public void testWhenAsIdentifier() {
assertTrue(false);
}
}

public void testTerminalTokensAPIs() {
char [][] ids = { "when".toCharArray(), "record".toCharArray(), "sealed".toCharArray(),
"permits".toCharArray(), "yield".toCharArray()};
int [] reskw = { TerminalTokens.TokenNameRestrictedIdentifierWhen,
TerminalTokens.TokenNameRestrictedIdentifierrecord,
TerminalTokens.TokenNameRestrictedIdentifiersealed,
TerminalTokens.TokenNameRestrictedIdentifierpermits,
TerminalTokens.TokenNameRestrictedIdentifierYield,};
int i = -1;
for (char [] id : ids) {
i++;
int t = TerminalTokens.getRestrictedKeyword(id);
assertTrue(t != TerminalTokens.TokenNameNotAToken);
assertTrue(TerminalTokens.isRestrictedKeyword(t));
assertTrue(t == reskw[i]);
}
assertTrue(TerminalTokens.getRestrictedKeyword("When".toCharArray()) == TerminalTokens.TokenNameNotAToken);
assertTrue(TerminalTokens.getRestrictedKeyword("blah".toCharArray()) == TerminalTokens.TokenNameNotAToken);
}
}
Loading

0 comments on commit 983c3bf

Please sign in to comment.