Skip to content

Commit

Permalink
Raise a diagnostic for qualified array access (#428)
Browse files Browse the repository at this point in the history
fixes #348
  • Loading branch information
MarkusAmshove authored Nov 23, 2023
2 parents ccacd35 + b3c5d54 commit 58b8029
Show file tree
Hide file tree
Showing 21 changed files with 482 additions and 28 deletions.
3 changes: 3 additions & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ allprojects {
}
}

compileJava.options.encoding = 'UTF-8'
compileTestJava.options.encoding = 'UTF-8'

tasks.withType(JavaCompile).configureEach {
options.release.set(JAVA_VERSION)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,4 +108,14 @@ public boolean completesParameter()
// don't.
return !isCurrentTokenKind(SyntaxKind.PERFORM) && !isCurrentTokenKind(SyntaxKind.CALLNAT) && !cursorIsExactlyOnCurrentToken();
}

public String previousTextsCombined()
{
var sb = new StringBuilder();
for (var previousText : previousTexts)
{
sb.append(previousText);
}
return sb.toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,13 @@ public List<CompletionItem> prepareCompletion(LanguageServerFile file, Completio
}

var isFilteredOnQualifiedNames = params.getContext().getTriggerKind() == CompletionTriggerKind.TriggerCharacter && ".".equals(params.getContext().getTriggerCharacter());
if (isFilteredOnQualifiedNames || completionContext.isCurrentTokenKind(SyntaxKind.LABEL_IDENTIFIER)) // Label identifier is what's lexed for incomplete qualification, e.g. GRP.
if (isFilteredOnQualifiedNames || completionContext.isCurrentTokenKind(SyntaxKind.LABEL_IDENTIFIER) || (completionContext.isCurrentTokenKind(SyntaxKind.DOT) && completionContext.isPreviousTokenKind(SyntaxKind.IDENTIFIER)))
{
assert completionContext.currentToken() != null;
var qualifiedNameFilter = completionContext.currentToken().symbolName();
assert completionContext.previousToken() != null;
var qualifiedNameFilter = completionContext.currentToken().kind() == SyntaxKind.DOT
? completionContext.previousTextsCombined()
: completionContext.currentToken().symbolName(); // label identifier
return findVariablesToComplete(module)
.filter(v -> v.qualifiedName().startsWith(qualifiedNameFilter))
.map(v -> toVariableCompletion(v, module, file, qualifiedNameFilter))
Expand Down Expand Up @@ -203,6 +206,7 @@ private CompletionItem toVariableCompletion(IVariableNode variableNode, INatural
try
{
var item = createCompletionItem(variableNode, file, module.referencableNodes(), !alreadyPresentText.isEmpty());
item.setLabel(item.getLabel().replace(alreadyPresentText, ""));
item.setInsertText(item.getInsertText().substring(alreadyPresentText.length()));
if (item.getKind() == CompletionItemKind.Variable)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ void filterVariableCompletionBasedOnTypedQualification()
END
""")
.assertDoesNotContainVariable("#GRP2.#VAR")
.assertContainsVariableCompleting("#GRP1.#VAR :(A10) (SUB)", "#VAR");
.assertContainsVariableCompleting("#VAR :(A10) (SUB)", "#VAR");
}

@Test
Expand All @@ -275,7 +275,7 @@ void filterVariableCompletionBasedOnTypedQualificationWhenVariablesAreNotAmbiguo
WRITE #GRP1.${}$
END
""")
.assertContainsVariableCompleting("#GRP1.#VARIABLE :(A10) (SUB)", "#VARIABLE");
.assertContainsVariableCompleting("#VARIABLE :(A10) (SUB)", "#VARIABLE");
}

@Test
Expand All @@ -293,7 +293,7 @@ void filterVariableCompletionBasedOnTypedQualificationOnTriggerChar()
END
""")
.assertDoesNotContainVariable("#GRP2.#VAR")
.assertContainsVariableCompleting("#GRP1.#VAR :(A10) (SUB)", "#VAR");
.assertContainsVariableCompleting("#VAR :(A10) (SUB)", "#VAR");
}

@Test
Expand All @@ -308,6 +308,6 @@ void filterVariableCompletionBasedOnTypedQualificationOnTriggerCharWhenVariables
WRITE #GRP1.${}$
END
""")
.assertContainsVariableCompleting("#GRP1.#VARIABLE :(A10) (SUB)", "#VARIABLE");
.assertContainsVariableCompleting("#VARIABLE :(A10) (SUB)", "#VARIABLE");
}
}
14 changes: 14 additions & 0 deletions libs/natparse/src/main/java/org/amshove/natparse/NodeUtil.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.amshove.natparse;

import org.amshove.natparse.lexing.SyntaxKind;
import org.amshove.natparse.lexing.SyntaxToken;
import org.amshove.natparse.natural.*;

Expand Down Expand Up @@ -331,4 +332,17 @@ public static ITokenNode findTokenNodeForToken(SyntaxToken token, ISyntaxTree tr

return null;
}

public static boolean containsTokenWithKind(ISyntaxNode node, SyntaxKind kind)
{
for (var descendant : node.descendants())
{
if (descendant instanceof ITokenNode tokenNode && tokenNode.token().kind() == kind)
{
return true;
}
}

return false;
}
}
33 changes: 28 additions & 5 deletions libs/natparse/src/main/java/org/amshove/natparse/lexing/Lexer.java
Original file line number Diff line number Diff line change
Expand Up @@ -894,6 +894,10 @@ private void consumeIdentifier()
{
if (scanner.peek() == '.')
{
if (!isValidIdentifierStart(scanner.peek(1)))
{
break;
}
isQualified = true;
}

Expand Down Expand Up @@ -943,14 +947,22 @@ && isValidIdentifierCharacter(scanner.peek()))
}
}

if (scanner.peek(-1) == '.')
if (scanner.peek() == '.' && (scanner.peek(1) == ')' || scanner.peek(1) == ' ' || (isInParens() && scanner.peek(1) == '/')))
{
scanner.advance();
createAndAdd(SyntaxKind.LABEL_IDENTIFIER);
return;
}
else
{
createAndAdd(SyntaxKind.IDENTIFIER);
}

createAndAdd(SyntaxKind.IDENTIFIER);
}

private boolean isValidIdentifierStart(char character)
{
return Character.isAlphabetic(character)
|| character == '&'
|| character == '#'
|| character == '+';
}

private boolean isValidIdentifierCharacter(char character)
Expand Down Expand Up @@ -1016,10 +1028,16 @@ private void consumeIdentifierOrKeyword()

while (!isLineEnd() && isNoWhitespace() && !scanner.isAtEnd() && isValidIdentifierCharacter(scanner.peek()))
{
var maybeAdabasSpecialIndexNotation = false;
// Characters from which we can be sure that we're dealing with an identifier
switch (scanner.peek())
{
case '.':
if (!isValidIdentifierStart(scanner.peek(1)))
{
maybeAdabasSpecialIndexNotation = true;
break;
}
isQualified = true;
case '@':
case '$':
Expand All @@ -1037,6 +1055,11 @@ private void consumeIdentifierOrKeyword()
break;
}

if (maybeAdabasSpecialIndexNotation && Character.isDigit(scanner.peek(1)))
{
break;
}

if (isInParens() && scanner.peek(-1) == '.' && (scanner.peek() == '/' || scanner.peek() == ')'))
{
createAndAdd(SyntaxKind.LABEL_IDENTIFIER);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package org.amshove.natparse.natural;

import org.amshove.natparse.ReadOnlyList;

public interface IAdabasIndexAccess extends IOperandNode
{
ReadOnlyList<IOperandNode> operands();
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@
import org.amshove.natparse.natural.*;
import org.amshove.natparse.natural.project.NaturalFileType;

import java.util.*;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Set;

abstract class AbstractParser<T>
{
Expand Down Expand Up @@ -1038,6 +1041,17 @@ protected IOperandNode consumeArrayAccess(BaseSyntaxNode reference) throws Parse
}

var access = consumeArithmeticExpression(reference);

if (access instanceof IVariableReferenceNode varRef && varRef.referencingToken().isQualified())
{
return consumeSpecialAdabasIndexAccess(reference, varRef, true);
}

if (peekKind(SyntaxKind.DOT) || (access instanceof ILiteralNode literal && literal.token().kind() == SyntaxKind.NUMBER_LITERAL && literal.token().source().contains(".")))
{
return consumeSpecialAdabasIndexAccess(reference, access, false);
}

if (peekKind(SyntaxKind.COLON))
{
return consumeRangedArrayAccess(reference, access);
Expand All @@ -1046,6 +1060,79 @@ protected IOperandNode consumeArrayAccess(BaseSyntaxNode reference) throws Parse
return access;
}

private IOperandNode consumeSpecialAdabasIndexAccess(BaseSyntaxNode originalReference, IOperandNode firstAccess, boolean needsSplit) throws ParseError
{
var access = new AdabasIndexAccess();
originalReference.addNode(access);

if (needsSplit)
{
// We have to split up an operand like #I.#K to actually be #I DOT #K, because that is the adabas notation
var qualifiedRef = (IVariableReferenceNode) firstAccess;

// Get rid of the previous node
((BaseSyntaxNode) qualifiedRef.parent()).removeNode((BaseSyntaxNode) qualifiedRef);
((BaseSyntaxNode) qualifiedRef).setParent(null);
unresolvedReferences.remove(qualifiedRef);

var qualifiedToken = qualifiedRef.referencingToken();
var tokenSplit = qualifiedToken.source().split("\\.");
var firstVarToken = new SyntaxToken(
SyntaxKind.IDENTIFIER,
qualifiedToken.offset(),
qualifiedToken.offsetInLine(),
qualifiedToken.line(),
tokenSplit[0],
qualifiedToken.filePath()
);
var dotToken = new SyntaxToken(
SyntaxKind.DOT,
qualifiedToken.offset() + tokenSplit[0].length(),
qualifiedToken.offsetInLine() + tokenSplit[0].length(),
qualifiedToken.line(),
".",
qualifiedToken.filePath()
);
var secondVarToken = new SyntaxToken(
SyntaxKind.IDENTIFIER,
dotToken.offset() + 1,
dotToken.offsetInLine() + 1,
qualifiedToken.line(),
tokenSplit[1],
qualifiedToken.filePath()
);

var firstRef = new VariableReferenceNode(firstVarToken);
var dot = new TokenNode(dotToken);
var secondRef = new VariableReferenceNode(secondVarToken);
access.addOperand(firstRef);
access.addNode(dot);
access.addOperand(secondRef);
unresolvedReferences.add(firstRef);
unresolvedReferences.add(secondRef);
}
else
{
access.setParent(firstAccess.parent());
((BaseSyntaxNode) firstAccess).setParent(access);
access.addOperand(firstAccess);
}

consumeOptionally(access, SyntaxKind.DOT);
while (!tokens.isAtEnd() && !peekKind(SyntaxKind.RPAREN))
{
if (peekKind(SyntaxKind.COLON) || peekKind(SyntaxKind.ASTERISK))
{
consume(access);
continue;
}

access.addOperand(consumeOperandNode(access));
}

return access;
}

protected IRangedArrayAccessNode consumeRangedArrayAccess(BaseSyntaxNode parent, IOperandNode lower) throws ParseError
{
var rangedAccess = new RangedArrayAccessNode();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package org.amshove.natparse.parsing;

import org.amshove.natparse.ReadOnlyList;
import org.amshove.natparse.natural.IAdabasIndexAccess;
import org.amshove.natparse.natural.IOperandNode;

import java.util.ArrayList;
import java.util.List;

class AdabasIndexAccess extends BaseSyntaxNode implements IAdabasIndexAccess
{
private final List<IOperandNode> operands = new ArrayList<>();

@Override
public ReadOnlyList<IOperandNode> operands()
{
return ReadOnlyList.from(operands);
}

void addOperand(IOperandNode operand)
{
addNode((BaseSyntaxNode) operand);
operands.add(operand);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ public void setParent(ISyntaxNode parent)
this.parent = parent;
}

void removeNode(BaseSyntaxNode node)
{
nodes.remove(node);
}

public ISyntaxNode parent()
{
return parent;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -261,8 +261,15 @@ private void resolveVariableReferences(StatementListParser statementParser, Natu
return;
}

var unresolvedAdabasArrayAccess = new ArrayList<ISymbolReferenceNode>();
for (var unresolvedReference : statementParser.getUnresolvedReferences())
{
if (unresolvedReference.parent() instanceof IAdabasIndexAccess)
{
unresolvedAdabasArrayAccess.add(unresolvedReference); // needs to be re-evaluated after, because it's parents need to be resolved
continue;
}

if (unresolvedReference.referencingToken().symbolName().startsWith("&")
|| (unresolvedReference.referencingToken().symbolName().contains(".")
&& unresolvedReference.referencingToken().symbolName().split("\\.")[1].startsWith("&")))
Expand Down Expand Up @@ -310,14 +317,46 @@ && tryFindAndReference(unresolvedReference.token().symbolName().substring(2), un

if (unresolvedReference.token().kind() == SyntaxKind.IDENTIFIER)
{
var diagnostic = ParserErrors.unresolvedReference(unresolvedReference);
reportUnresolvedReference(module, unresolvedReference);
}
}

for (var unresolvedReference : unresolvedAdabasArrayAccess)
{
if (unresolvedReference.parent()instanceof IAdabasIndexAccess adabasIndexAccess
&& adabasIndexAccess.parent()instanceof IVariableReferenceNode arrayRef
&& arrayRef.reference()instanceof IVariableNode resolvedArray
&& !resolvedArray.isInView())
{
var diagnostic = ParserErrors.variableQualificationNotAllowedHere(
"Variable qualification is not allowed when not referring to a database array",
adabasIndexAccess.diagnosticPosition()
);

if (!diagnostic.filePath().equals(module.file().getPath()))
{
diagnostic = diagnostic.relocate(unresolvedReference.diagnosticPosition());
}
module.addDiagnostic(diagnostic);
}
else
{
if (!tryFindAndReference(unresolvedReference.token().symbolName(), unresolvedReference, defineData, module))
{
reportUnresolvedReference(module, unresolvedReference);
}
}
}
}

private void reportUnresolvedReference(NaturalModule module, ISymbolReferenceNode unresolvedReference)
{
var diagnostic = ParserErrors.unresolvedReference(unresolvedReference);
if (!diagnostic.filePath().equals(module.file().getPath()))
{
diagnostic = diagnostic.relocate(unresolvedReference.diagnosticPosition());
}
module.addDiagnostic(diagnostic);
}

private boolean tryFindAndReference(String symbolName, ISymbolReferenceNode referenceNode, IDefineData defineData, NaturalModule module)
Expand Down
Loading

0 comments on commit 58b8029

Please sign in to comment.