Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check literal conditionals in DECIDE blocks always resolving to false #234

Merged
merged 6 commits into from
Jun 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
package org.amshove.natlint.analyzers;

import org.amshove.natlint.api.AbstractAnalyzer;
import org.amshove.natlint.api.DiagnosticDescription;
import org.amshove.natlint.api.IAnalyzeContext;
import org.amshove.natlint.api.ILinterContext;
import org.amshove.natparse.DiagnosticSeverity;
import org.amshove.natparse.ReadOnlyList;
import org.amshove.natparse.natural.*;

public class ConditionAlwaysFalseAnalyzer extends AbstractAnalyzer
{
public static final DiagnosticDescription CONDITION_ALWAYS_FALSE = DiagnosticDescription.create(
"NL017",
"Condition is always false: %s",
DiagnosticSeverity.WARNING
);

@Override
public ReadOnlyList<DiagnosticDescription> getDiagnosticDescriptions()
{
return ReadOnlyList.of(CONDITION_ALWAYS_FALSE);
}

@Override
public void initialize(ILinterContext context)
{
context.registerNodeAnalyzer(IDecideOnBranchNode.class, this::analyzeDecideBranch);
}

private void analyzeDecideBranch(ISyntaxNode node, IAnalyzeContext context)
{
var branch = (IDecideOnBranchNode) node;
var decide = ((IDecideOnNode) branch.parent());

if (!(decide.operand()instanceof IVariableReferenceNode varRef)
|| !(varRef.reference()instanceof ITypedVariableNode typedTarget)
|| typedTarget.type() == null)
{
return;
}

for (var value : branch.values())
{
if (!(value instanceof ILiteralNode literal))
{
continue;
}

checkLiteralType(context, typedTarget, literal);
}
}

private static void checkLiteralType(IAnalyzeContext context, ITypedVariableNode typedTarget, ILiteralNode literal)
{
var inferredType = literal.inferType(typedTarget.type().format());
if (!inferredType.fitsInto(typedTarget.type()))
{
context.report(
CONDITION_ALWAYS_FALSE.createFormattedDiagnostic(
literal.position(), "Inferred type %s does not fit into %s %s"
.formatted(
inferredType.toShortString(),
typedTarget.declaration().symbolName(),
typedTarget.type().toShortString()
)
)
);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
package org.amshove.natlint.analyzers;

import org.amshove.natlint.linter.AbstractAnalyzerTest;
import org.junit.jupiter.api.Test;

class ConditionAlwaysFalseAnalyzerShould extends AbstractAnalyzerTest
{
protected ConditionAlwaysFalseAnalyzerShould()
{
super(new ConditionAlwaysFalseAnalyzer());
}

@Test
void reportADecideConditionThatCanNeverBeMet()
{
testDiagnostics(
"""
DEFINE DATA LOCAL
1 #AVAR (A1)
END-DEFINE
DECIDE ON FIRST VALUE OF #AVAR
VALUE 'Hello'
IGNORE
NONE
IGNORE
END-DECIDE
END
""",
expectDiagnostic(4, ConditionAlwaysFalseAnalyzer.CONDITION_ALWAYS_FALSE)
);
}

@Test
void notReportADecideConditionWhichFitsTrimmed()
{
testDiagnostics(
"""
DEFINE DATA LOCAL
1 #AVAR (A1)
END-DEFINE
DECIDE ON FIRST VALUE OF #AVAR
VALUE 'H '
IGNORE
NONE
IGNORE
END-DECIDE
END
""",
expectNoDiagnostic(4, ConditionAlwaysFalseAnalyzer.CONDITION_ALWAYS_FALSE)
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,17 @@ public interface IDataType
boolean hasDynamicLength();

/**
* Determines if this type fits into the given type. Implicit conversion is taken into account.
* Determines if this type fits into the given type. Implicit conversion is taken into account.</br>
* <strong>This does not compare by byte size</strong>
*/
default boolean fitsInto(IDataType target)
{
var bytesFit = this.byteSize() <= target.byteSize();
var ourLength = this.hasDynamicLength() ? ONE_GIGABYTE : length();
var theirLength = target.hasDynamicLength() ? ONE_GIGABYTE : target.length();
var lengthFits = ourLength <= theirLength;
var formatIsCompatible = hasCompatibleFormat(target);

return bytesFit && formatIsCompatible;
return lengthFits && formatIsCompatible;
}

/**
Expand Down Expand Up @@ -120,7 +123,7 @@ private int calculateNumericSize()
return Math.max(1, digitsBeforeDecimalPoint + digitsAfterDecimalPoint);
}

private int calculatePackedSize()
default int calculatePackedSize()
{
return Math.max(1, (int) (Math.round((calculateNumericSize() + 1) / 2.0)));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@

public interface ILiteralNode extends ITokenNode, IOperandNode
{
IDataType dataType();
IDataType inferType(DataFormat targetType);
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public AttributeNode(SyntaxToken token)
}

@Override
public IDataType dataType()
public IDataType inferType(DataFormat targetFormat)
{
return DATA_TYPE;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,26 @@

class LiteralNode extends TokenNode implements ILiteralNode
{
private final IDataType dataType;

public LiteralNode(SyntaxToken token)
{
super(token);
}

dataType = switch (token.kind())
@Override
public IDataType inferType(DataFormat targetFormat)
{
var token = token();
return switch (token.kind())
{
case STRING_LITERAL -> new LiteralType(DataFormat.ALPHANUMERIC, token.stringValue().length());
case NUMBER_LITERAL -> new LiteralType(DataFormat.NUMERIC, getNumericLiteralLength(token.source()));
case STRING_LITERAL -> new LiteralType(DataFormat.ALPHANUMERIC, token.stringValue().trim().length());
case NUMBER_LITERAL -> switch (targetFormat)
{
case BINARY -> new LiteralType(DataFormat.BINARY, getIntegerLiteralLength(token.source()));
case INTEGER -> new LiteralType(DataFormat.INTEGER, getIntegerLiteralLength(token.source()));
case FLOAT -> new LiteralType(DataFormat.FLOAT, getIntegerLiteralLength(token.source()));
case PACKED -> new LiteralType(DataFormat.PACKED, getNumericLiteralLength(token.source()));
default -> new LiteralType(DataFormat.NUMERIC, getNumericLiteralLength(token.source()));
};
case DATE_LITERAL -> new LiteralType(DataFormat.DATE, 4);
case TIME_LITERAL, EXTENDED_TIME_LITERAL -> new LiteralType(DataFormat.TIME, 7);

Expand Down Expand Up @@ -46,10 +56,32 @@ private double getNumericLiteralLength(String source)
return Double.parseDouble("%s.%s".formatted(split[0].length(), split[1].length())); // there must be a smarter way
}

@Override
public IDataType dataType()
private double getIntegerLiteralLength(String source)
{
return dataType;
if (source.contains(".") || source.contains(","))
{
return getNumericLiteralLength(source);
}

var byteSize = Long.toBinaryString(Long.parseLong(source)).length() / 8.0;
if (byteSize < 1)
{
return 1;
}

if (byteSize < 2)
{
return 2;
}

if (byteSize > 4)
{
// for too big literals, round the bytes up.
// I5 isn't a valid type, but will result in the correct type error.
return (int) (byteSize + 1);
}

return 4;
}

record LiteralType(DataFormat format, double length) implements IDataType
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ private void checkSystemFunctionParameter(ISystemFunctionNode sysFuncNode)
{
for (var parameter : sysFuncNode.parameter())
{
var type = inferDataType(parameter);
var type = inferDataType(parameter, DataFormat.ALPHANUMERIC);
if (type == null)
{
continue;
Expand All @@ -112,7 +112,7 @@ private void checkSystemFunctionParameter(ISystemFunctionNode sysFuncNode)
{
var parameter = sysFuncNode.parameter().first();

var type = inferDataType(parameter);
var type = inferDataType(parameter, DataFormat.ALPHANUMERIC);
if (type != null && type.format() != DataFormat.NONE && type.format() != DataFormat.ALPHANUMERIC && type.format() != DataFormat.UNICODE && type.format() != DataFormat.BINARY)
{
report(ParserErrors.typeMismatch("Parameter to *TRIM must be of type A, B or U but is %s".formatted(type.toShortString()), parameter));
Expand All @@ -133,7 +133,7 @@ private void checkDecideOnBranches(IDecideOnNode decideOn)
{
for (var value : branch.values())
{
var inferredType = inferDataType(value);
var inferredType = inferDataType(value, typedTarget.type().format());
if (inferredType.format() != DataFormat.NONE && !inferredType.hasSameFamily(typedTarget.type()))
{
report(
Expand Down Expand Up @@ -421,7 +421,7 @@ private boolean containsDynamicDimension(IRangedArrayAccessNode ranged)
|| upperLiteral.token().kind() == SyntaxKind.ASTERISK);
}

private IDataType inferDataType(IOperandNode operand)
private IDataType inferDataType(IOperandNode operand, DataFormat targetFormat)
{
if (operand instanceof IVariableReferenceNode variable && variable.reference()instanceof ITypedVariableNode typedRef)
{
Expand All @@ -430,7 +430,7 @@ private IDataType inferDataType(IOperandNode operand)

if (operand instanceof ILiteralNode literal)
{
return literal.dataType();
return literal.inferType(targetFormat);
}

if (operand instanceof ISystemFunctionNode sysFunction)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -723,32 +723,32 @@ void parseConditionsWithDateLiterals()
{
var criteria = assertParsesCriteria("#VAR < D'1990-01-01'", IRelationalCriteriaNode.class);
assertThat(assertNodeType(criteria.right(), ILiteralNode.class).token().kind()).isEqualTo(SyntaxKind.DATE_LITERAL);
assertThat(assertNodeType(criteria.right(), ILiteralNode.class).dataType().format()).isEqualTo(DataFormat.DATE);
assertThat(assertNodeType(criteria.right(), ILiteralNode.class).inferType(DataFormat.DATE).format()).isEqualTo(DataFormat.DATE);
}

@Test
void parseConditionsWithTimeLiterals()
{
var criteria = assertParsesCriteria("#VAR < T'15:00:00'", IRelationalCriteriaNode.class);
assertThat(assertNodeType(criteria.right(), ILiteralNode.class).token().kind()).isEqualTo(SyntaxKind.TIME_LITERAL);
assertThat(assertNodeType(criteria.right(), ILiteralNode.class).dataType().format()).isEqualTo(DataFormat.TIME);
assertThat(assertNodeType(criteria.right(), ILiteralNode.class).inferType(DataFormat.TIME).format()).isEqualTo(DataFormat.TIME);
}

@Test
void parseConditionsWithExtendedTimeLiterals()
{
var criteria = assertParsesCriteria("#VAR < E'2010-02-02 15:00:00'", IRelationalCriteriaNode.class);
assertThat(assertNodeType(criteria.right(), ILiteralNode.class).token().kind()).isEqualTo(SyntaxKind.EXTENDED_TIME_LITERAL);
assertThat(assertNodeType(criteria.right(), ILiteralNode.class).dataType().format()).isEqualTo(DataFormat.TIME);
assertThat(assertNodeType(criteria.right(), ILiteralNode.class).inferType(DataFormat.TIME).format()).isEqualTo(DataFormat.TIME);
}

@Test
void parseConditionsWithHexLiterals()
{
var criteria = assertParsesCriteria("#VAR = H'0A'", IRelationalCriteriaNode.class);
assertThat(assertNodeType(criteria.right(), ILiteralNode.class).token().kind()).isEqualTo(SyntaxKind.HEX_LITERAL);
assertThat(assertNodeType(criteria.right(), ILiteralNode.class).dataType().format()).isEqualTo(DataFormat.ALPHANUMERIC);
assertThat(assertNodeType(criteria.right(), ILiteralNode.class).dataType().length()).isEqualTo(1);
assertThat(assertNodeType(criteria.right(), ILiteralNode.class).inferType(DataFormat.ALPHANUMERIC).format()).isEqualTo(DataFormat.ALPHANUMERIC);
assertThat(assertNodeType(criteria.right(), ILiteralNode.class).inferType(DataFormat.ALPHANUMERIC).length()).isEqualTo(1);
}

@Test
Expand Down
Loading