Skip to content

Commit

Permalink
Fix NPE when inline variables are declared in sub scopes
Browse files Browse the repository at this point in the history
  • Loading branch information
@gquerret
gquerret committed Nov 29, 2021
1 parent 79a087b commit edc6baf
Show file tree
Hide file tree
Showing 5 changed files with 137 additions and 124 deletions.
2 changes: 1 addition & 1 deletion proparse/src/main/antlr4/org/prorefactor/proparse/antlr4/Proparse.g4
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1329,7 +1329,7 @@ datatypeVar:
| UNSIGNEDBYTE
| UNSIGNEDSHORT
| UNSIGNEDINTEGER
| { support.abbrevDatatype(_input.LT(1).getText()) !=0 }? id=ID // Like 'i' for INTEGER or 'de' for DECIMAL
| { ABLNodeType.abbrevDatatype(_input.LT(1).getText()) != ABLNodeType.INVALID_NODE }? id=ID // Like 'i' for INTEGER or 'de' for DECIMAL
| { !support.isDataTypeVariable(_input.LT(1)) }? typeName
;

Expand Down
37 changes: 37 additions & 0 deletions proparse/src/main/java/org/prorefactor/core/ABLNodeType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2081,6 +2081,43 @@ public static DataType getDataType(int nodeType) {
}
}

/**
* An AS phrase allows further abbreviations on the datatype names. Input a token's text, this returns 0 if it is not
* a datatype abbreviation, otherwise returns the integer token type for the abbreviation. Here's the normal keyword
* abbreviation, with what AS phrase allows:
* <ul>
* <li>char: c
* <li>date: da
* <li>dec: de
* <li>int: i
* <li>logical: l
* <li>recid: rec
* <li>rowid: rowi
* <li>widget-h: widg
* </ul>
*/
public static ABLNodeType abbrevDatatype(String text) {
String s = text.toLowerCase();
if ("cha".startsWith(s))
return ABLNodeType.CHARACTER;
if ("da".equals(s) || "dat".equals(s))
return ABLNodeType.DATE;
if ("de".equals(s))
return ABLNodeType.DECIMAL;
if ("i".equals(s) || "in".equals(s))
return ABLNodeType.INTEGER;
if ("logical".startsWith(s))
return ABLNodeType.LOGICAL;
if ("rec".equals(s) || "reci".equals(s))
return ABLNodeType.RECID;
if ("rowi".equals(s))
return ABLNodeType.ROWID;
if ("widget-h".startsWith(s) && s.length() >= 4)
return ABLNodeType.WIDGETHANDLE;

return ABLNodeType.INVALID_NODE;
}

private static void generateKeywordsG4(final PrintStream out) {
out.println("// Generated file - Do not manually edit");
out.println();
Expand Down
11 changes: 7 additions & 4 deletions proparse/src/main/java/org/prorefactor/proparse/JPNodeVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1163,7 +1163,7 @@ else if (builder.getNodeType() == ABLNodeType.ROW)
else if (builder.getNodeType() == ABLNodeType.WIDGET)
builder.changeType(ABLNodeType.WIDGETHANDLE);
else if (ctx.id != null)
builder.changeType(ABLNodeType.getNodeType(ParserSupport.abbrevDatatype(ctx.id.getText())));
builder.changeType(ABLNodeType.abbrevDatatype(ctx.id.getText()));

return builder.setRuleNode(ctx);
}
Expand Down Expand Up @@ -2086,7 +2086,10 @@ public Builder visitNullPhrase(NullPhraseContext ctx) {

@Override
public Builder visitOnStatement(OnStatementContext ctx) {
return createStatementTreeFromFirstNode(ctx).setBlock(true);
support.visitorEnterScope(ctx);
Builder builder = createStatementTreeFromFirstNode(ctx).setBlock(true);
support.visitorExitScope(ctx);
return builder;
}

@Override
Expand Down Expand Up @@ -2724,7 +2727,7 @@ public Builder visitTriggerOfSub1(TriggerOfSub1Context ctx) {

@Override
public Builder visitTriggerOfSub2(TriggerOfSub2Context ctx) {
support.defVar(ctx.id.getText());
support.defVar(ctx.id.getText()); // FIXME Is that really needed at this stage ?
return createTreeFromFirstNode(ctx).setRuleNode(ctx);
}

Expand All @@ -2736,7 +2739,7 @@ public Builder visitTriggerTableLabel(TriggerTableLabelContext ctx) {
@Override
public Builder visitTriggerOld(TriggerOldContext ctx) {
Builder node = createTreeFromFirstNode(ctx).setRuleNode(ctx);
support.defVar(ctx.id.getText());
support.defVar(ctx.id.getText()); // FIXME Is that really needed at this stage ?
return node;
}

Expand Down
195 changes: 76 additions & 119 deletions proparse/src/main/java/org/prorefactor/proparse/support/ParserSupport.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import org.antlr.v4.runtime.tree.ParseTreeProperty;
import org.prorefactor.core.ABLNodeType;
import org.prorefactor.core.JPNode;
import org.prorefactor.proparse.antlr4.Proparse;
import org.prorefactor.proparse.support.SymbolScope.FieldType;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -92,59 +91,6 @@ public IProparseEnvironment getProparseSession() {
// Functions triggered from proparse.g4
// ************************************

/**
* An AS phrase allows further abbreviations on the datatype names. Input a token's text, this returns 0 if it is not
* a datatype abbreviation, otherwise returns the integer token type for the abbreviation. Here's the normal keyword
* abbreviation, with what AS phrase allows:
* <ul>
* <li>char: c
* <li>date: da
* <li>dec: de
* <li>int: i
* <li>logical: l
* <li>recid: rec
* <li>rowid: rowi
* <li>widget-h: widg
* </ul>
*/
public static int abbrevDatatype(String text) {
String s = text.toLowerCase();
if ("cha".startsWith(s))
return Proparse.CHARACTER;
if ("da".equals(s) || "dat".equals(s))
return Proparse.DATE;
if ("de".equals(s))
return Proparse.DECIMAL;
if ("i".equals(s) || "in".equals(s))
return Proparse.INTEGER;
if ("logical".startsWith(s))
return Proparse.LOGICAL;
if ("rec".equals(s) || "reci".equals(s))
return Proparse.RECID;
if ("rowi".equals(s))
return Proparse.ROWID;
if ("widget-h".startsWith(s) && s.length() >= 4)
return Proparse.WIDGETHANDLE;
return 0;
}

// TEMP-ANTLR4
public void visitorEnterScope(RuleContext ctx) {
SymbolScope scope = innerScopesMap.get(ctx);
if (scope != null) {
currentScope = scope;
}
}

// TEMP-ANTLR4
public void visitorExitScope(RuleContext ctx) {
SymbolScope scope = innerScopesMap.get(ctx);
if (scope != null) {
currentScope = currentScope.getSuperScope();
}
}

// TEMP-ANTLR4
public void addInnerScope(RuleContext ctx) {
currentScope = new SymbolScope(session, currentScope);
innerScopes.add(currentScope);
Expand Down Expand Up @@ -235,39 +181,11 @@ public boolean recordSemanticPredicate(Token lt1, Token lt2, Token lt3) {
return (schemaTablePriority ? isTableSchemaFirst(recname.toLowerCase()) : isTable(recname.toLowerCase())) != null;
}

// *******************************************
// End of functions triggered from proparse.g4
// *******************************************

public void pushNode(ParseTree ctx, JPNode node) {
nodes.put(ctx, node);
}

public JPNode getNode(ParseTree ctx) {
return nodes.get(ctx);
}

public void pushRecordExpression(RuleContext ctx, String recName) {
recordExpressions.put(ctx, schemaTablePriority ? currentScope.isTableSchemaFirst(recName.toLowerCase())
: currentScope.isTable(recName.toLowerCase()));
}

public FieldType getRecordExpression(RuleContext ctx) {
return recordExpressions.get(ctx);
}

public void clearRecordExpressions() {
recordExpressions = new ParseTreeProperty<>();
}

public FieldType isTable(String inName) {
return currentScope.isTable(inName);
}

public FieldType isTableSchemaFirst(String inName) {
return currentScope.isTableSchemaFirst(inName);
}

/** Returns true if the lookahead is a table name, and not a var name. */
public boolean isTableName(Token token) {
int numDots = CharMatcher.is('.').countIn(token.getText());
Expand All @@ -286,44 +204,12 @@ public boolean isVar(String name) {
return currentScope.isVariable(name);
}

public boolean isInlineVar(String name) {
return currentScope.isInlineVariable(name);
}

public int isMethodOrFunc(String name) {
// Methods and user functions are only at the "unit" (class) scope.
// Methods can also be inherited from superclasses.
return unitScope.isMethodOrFunction(name);
}

public int isMethodOrFunc(Token token) {
if (token == null)
return 0;
return unitScope.isMethodOrFunction(token.getText());
}

/**
* @return True if parsing a class or interface
*/
public boolean isClass() {
return !Strings.isNullOrEmpty(className);
}

/**
* @return True if parsing an interface
*/
public boolean isInterface() {
return unitIsInterface;
}

public boolean isEnum() {
return unitIsEnum;
}

public boolean isSchemaTablePriority() {
return schemaTablePriority;
}

public void setSchemaTablePriority(boolean priority) {
this.schemaTablePriority = priority;
}
Expand All @@ -344,11 +230,6 @@ public boolean unknownMethodCallsAllowed() {
return allowUnknownMethodCalls;
}

// TODO Speed issue in this function, multiplied JPNode tree generation time by a factor 10
public String lookupClassName(String text) {
return classFinder.lookup(text);
}

public boolean hasHiddenBefore(TokenStream stream) {
int currIndex = stream.index();
// Obviously no hidden token for first token
Expand All @@ -366,4 +247,80 @@ public boolean hasHiddenAfter(TokenStream stream) {
// Otherwise see if token is in different channel
return stream.get(currIndex + 1).getChannel() != Token.DEFAULT_CHANNEL;
}

/**
* @return True if parsing a class or interface
*/
public boolean isClass() {
return !Strings.isNullOrEmpty(className);
}

/**
* @return True if parsing an interface
*/
public boolean isInterface() {
return unitIsInterface;
}

public boolean isEnum() {
return unitIsEnum;
}

private FieldType isTable(String inName) {
return currentScope.isTable(inName);
}

private FieldType isTableSchemaFirst(String inName) {
return currentScope.isTableSchemaFirst(inName);
}

// *******************************************
// End of functions triggered from proparse.g4
// *******************************************

public void pushNode(ParseTree ctx, JPNode node) {
nodes.put(ctx, node);
}

public JPNode getNode(ParseTree ctx) {
return nodes.get(ctx);
}

public void visitorEnterScope(RuleContext ctx) {
SymbolScope scope = innerScopesMap.get(ctx);
if (scope != null) {
currentScope = scope;
}
}

public void visitorExitScope(RuleContext ctx) {
SymbolScope scope = innerScopesMap.get(ctx);
if (scope != null) {
currentScope = currentScope.getSuperScope();
}
}

public FieldType getRecordExpression(RuleContext ctx) {
return recordExpressions.get(ctx);
}

public void clearRecordExpressions() {
recordExpressions = new ParseTreeProperty<>();
}

public boolean isInlineVar(String name) {
return currentScope.isInlineVariable(name);
}

public int isMethodOrFunc(String name) {
// Methods and user functions are only at the "unit" (class) scope.
// Methods can also be inherited from superclasses.
return unitScope.isMethodOrFunction(name);
}

// TODO Speed issue in this function, multiplied JPNode tree generation time by a factor 10
public String lookupClassName(String text) {
return classFinder.lookup(text);
}

}
16 changes: 16 additions & 0 deletions proparse/src/test/java/org/prorefactor/proparse/ExpressionEngineTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,22 @@ public void testInlineVariable03() {
assertEquals(exp.getDataType().getPrimitive(), PrimitiveDataType.DECIMAL);
}

@Test
public void testInlineVariable04() {
// Not a good test case, this has to be removed. The way inline variables are handled has to be rewritten,
// as it is the only reason why we have to maintain the ParseSupport.currentScope object from JPNodeVisitor
// This test case just ensures that the scope is set correctly, as a bug was detected late in the dev cycle
// that broke the parser with NPE in some inline variables cases
ParseUnit unit01 = new ParseUnit("procedure p1: message 'xx' update lVar as log. end.", session);
unit01.treeParser01();
ParseUnit unit02 = new ParseUnit("function f1 returns char(): message 'xx' update lVar as log. end.", session);
unit02.treeParser01();
ParseUnit unit03 = new ParseUnit("on choose of btn1 do: message 'xx' update lVar as log. end.", session);
unit03.treeParser01();
ParseUnit unit04 = new ParseUnit("class cls1: method public void m1(): message 'xx' update lVar as log. end. end.", session);
unit04.treeParser01();
}

private void testSimpleExpression(String code, DataType expected) {
ParseUnit unit01 = new ParseUnit(code, session);
unit01.treeParser01();
Expand Down

0 comments on commit edc6baf

Please sign in to comment.