-
Notifications
You must be signed in to change notification settings - Fork 3
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Migrate value truncation check to an Analyzer (#398)
closes #347
- Loading branch information
1 parent
03c65fa
commit d22575b
Showing
10 changed files
with
199 additions
and
69 deletions.
There are no files selected for viewing
83 changes: 83 additions & 0 deletions
83
libs/natlint/src/main/java/org/amshove/natlint/analyzers/ValueTruncationAnalyzer.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| 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 ValueTruncationAnalyzer extends AbstractAnalyzer | ||
| { | ||
| public static final DiagnosticDescription VALUE_TRUNCATED = DiagnosticDescription.create( | ||
| "NL021", | ||
| "Value is truncated from %s to %s at runtime. Extend the target variable or remove the truncated parts from this literal.", | ||
| DiagnosticSeverity.WARNING | ||
| ); | ||
|
|
||
| @Override | ||
| public ReadOnlyList<DiagnosticDescription> getDiagnosticDescriptions() | ||
| { | ||
| return ReadOnlyList.of(VALUE_TRUNCATED); | ||
| } | ||
|
|
||
| @Override | ||
| public void initialize(ILinterContext context) | ||
| { | ||
| context.registerNodeAnalyzer(IAssignmentStatementNode.class, this::analyzeAssign); | ||
| context.registerNodeAnalyzer(ITypedVariableNode.class, this::analyzeInitialValue); | ||
| } | ||
|
|
||
| private void analyzeInitialValue(ISyntaxNode typedVariable, IAnalyzeContext context) | ||
| { | ||
| var typedVar = (ITypedVariableNode) typedVariable; | ||
| var initialNode = typedVar.type().initialValue(); | ||
| if (initialNode == null || typedVar.type().hasDynamicLength() || !(initialNode instanceof ILiteralNode || initialNode instanceof IStringConcatOperandNode)) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| var inferredInitialType = initialNode instanceof ILiteralNode literal | ||
| ? literal.reInferType(typedVar.type()) | ||
| : ((IStringConcatOperandNode) initialNode).inferType(); | ||
|
|
||
| checkTruncation(inferredInitialType, typedVar.type(), initialNode, context); | ||
| } | ||
|
|
||
| private void analyzeAssign(ISyntaxNode assignNode, IAnalyzeContext context) | ||
| { | ||
| var assignment = (IAssignmentStatementNode) assignNode; | ||
|
|
||
| if (!(assignment.target()instanceof IVariableReferenceNode targetRef | ||
| && targetRef.reference()instanceof ITypedVariableNode typedTarget | ||
| && typedTarget.type() != null)) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| if (assignment.operand()instanceof ILiteralNode literal) | ||
| { | ||
| checkTruncation(literal.reInferType(typedTarget.type()), typedTarget.type(), assignment.operand(), context); | ||
| } | ||
| } | ||
|
|
||
| private void checkTruncation(IDataType operandType, IDataType targetType, ISyntaxNode location, IAnalyzeContext context) | ||
| { | ||
| if (!operandType.hasCompatibleFormat(targetType)) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| if (!operandType.fitsInto(targetType)) | ||
| { | ||
| context.report( | ||
| VALUE_TRUNCATED.createFormattedDiagnostic( | ||
| location.diagnosticPosition(), | ||
| operandType.toShortString(), | ||
| targetType.toShortString() | ||
| ) | ||
| ); | ||
| } | ||
| } | ||
| } |
76 changes: 76 additions & 0 deletions
76
libs/natlint/src/test/java/org/amshove/natlint/analyzers/ValueTruncationAnalyzerShould.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| package org.amshove.natlint.analyzers; | ||
|
|
||
| import org.amshove.natlint.linter.AbstractAnalyzerTest; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| class ValueTruncationAnalyzerShould extends AbstractAnalyzerTest | ||
| { | ||
| @Test | ||
| void raiseADiagnosticWhenAConstInitializerIsTruncatedForCompatibleFormats() | ||
| { | ||
| testDiagnostics(""" | ||
| DEFINE DATA LOCAL | ||
| 1 #C-CONST (A1) CONST<20> | ||
| END-DEFINE | ||
| """, expectDiagnostic(1, ValueTruncationAnalyzer.VALUE_TRUNCATED)); | ||
| } | ||
|
|
||
| @Test | ||
| void raiseADiagnosticWhenAnInitInitializerIsTruncatedForCompatibleFormats() | ||
| { | ||
| testDiagnostics(""" | ||
| DEFINE DATA LOCAL | ||
| 1 #C-CONST (A1) INIT<20> | ||
| END-DEFINE | ||
| """, expectDiagnostic(1, ValueTruncationAnalyzer.VALUE_TRUNCATED)); | ||
| } | ||
|
|
||
| @Test | ||
| void reportADiagnosticWhenNumericAssignmentValuesGetTruncated() | ||
| { | ||
| testDiagnostics( | ||
| """ | ||
| DEFINE DATA LOCAL | ||
| 1 #CONST-N1-I4 (I4) CONST<1> | ||
| 1 #N1 (N1) | ||
| 1 #I1 (I1) | ||
| END-DEFINE | ||
| #N1 := 23 | ||
| #I1 := 128 | ||
| END | ||
| """, | ||
| expectDiagnostic(5, ValueTruncationAnalyzer.VALUE_TRUNCATED), | ||
| expectDiagnostic(6, ValueTruncationAnalyzer.VALUE_TRUNCATED) | ||
| ); | ||
| } | ||
|
|
||
| @Test | ||
| void reportADiagnosticWhenNumericAssignmentValuesGetTruncatedWithCompatibleTargetFormat() | ||
| { | ||
| testDiagnostics(""" | ||
| DEFINE DATA LOCAL | ||
| 1 #CONST-N1-I4 (I4) CONST<1> | ||
| 1 #A1 (A1) | ||
| END-DEFINE | ||
| #A1 := 10 | ||
| END | ||
| """, expectDiagnostic(4, ValueTruncationAnalyzer.VALUE_TRUNCATED)); | ||
| } | ||
|
|
||
| @Test | ||
| void reportADiagnosticWhenAlphanumericAssignmentValuesGetTruncated() | ||
| { | ||
| testDiagnostics(""" | ||
| DEFINE DATA LOCAL | ||
| 1 #A1 (A1) | ||
| END-DEFINE | ||
| #A1 := 'AB' | ||
| END | ||
| """, expectDiagnostic(3, ValueTruncationAnalyzer.VALUE_TRUNCATED)); | ||
| } | ||
|
|
||
| protected ValueTruncationAnalyzerShould() | ||
| { | ||
| super(new ValueTruncationAnalyzer()); | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| name: Literal value truncated at runtime | ||
| priority: MINOR | ||
| tags: pitfall,confusing | ||
| type: CODE_SMELL | ||
| description: | ||
| The value of this literal is truncated at runtime. This will raise unexpected results. | ||
|
|
||
| == Non compliant | ||
|
|
||
| `` | ||
| DEFINE DATA LOCAL | ||
| 1 #VAR (A1) | ||
| END-DEFINE | ||
|
|
||
| #VAR := 'Hi' | ||
| WRITE #VAR | ||
| END | ||
| `` | ||
|
|
||
| == Compliant | ||
|
|
||
| `` | ||
| DEFINE DATA LOCAL | ||
| 1 #VAR (A1) | ||
| END-DEFINE | ||
|
|
||
| #VAR := 'H' | ||
| WRITE #VAR | ||
| END | ||
| `` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,30 +1,6 @@ | ||
| name: Literal value truncated at runtime | ||
| priority: MINOR | ||
| tags: pitfall,confusing | ||
| type: CODE_SMELL | ||
| name: EM, HD, PM not allowed in this scope | ||
| priority: BLOCKER | ||
| tags: natparse-internal,compile-time | ||
| type: BUG | ||
| description: | ||
| The value of this literal is truncated at runtime. This will raise unexpected results. | ||
|
|
||
| == Non compliant | ||
|
|
||
| `` | ||
| DEFINE DATA LOCAL | ||
| 1 #VAR (A1) | ||
| END-DEFINE | ||
|
|
||
| #VAR := 'Hi' | ||
| WRITE #VAR | ||
| END | ||
| `` | ||
|
|
||
| == Compliant | ||
|
|
||
| `` | ||
| DEFINE DATA LOCAL | ||
| 1 #VAR (A1) | ||
| END-DEFINE | ||
|
|
||
| #VAR := 'H' | ||
| WRITE #VAR | ||
| END | ||
| `` | ||
| ``EM, HD, PM`` is not allowed in ``PARAMETER`` scope. |
This file was deleted.
Oops, something went wrong.