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

Raise a diagnostic for unnecessary IGNORE statements #490

Merged
merged 3 commits into from
Jul 23, 2024
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,48 @@
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.NodeUtil;
import org.amshove.natparse.ReadOnlyList;
import org.amshove.natparse.natural.IIgnoreNode;
import org.amshove.natparse.natural.IStatementListNode;
import org.amshove.natparse.natural.ISyntaxNode;

public class UnnecessaryIgnoreAnalyzer extends AbstractAnalyzer
{
public static final DiagnosticDescription UNNECESSARY_IGNORE = DiagnosticDescription.create(
"NL025",
"IGNORE is unnecessary",
DiagnosticSeverity.INFO
);

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

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

private void analyzeIgnore(ISyntaxNode node, IAnalyzeContext context)
{
var ignore = (IIgnoreNode) node;

if (!(NodeUtil.findFirstParentOfType(ignore, IStatementListNode.class)instanceof IStatementListNode parent))
{
return;
}

if (parent.statements().size() > 1)
{
context.report(UNNECESSARY_IGNORE.createDiagnostic(ignore));
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
package org.amshove.natlint.analyzers;

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

class UnnecessaryIgnoreAnalyzerShould extends AbstractAnalyzerTest
{
protected UnnecessaryIgnoreAnalyzerShould()
{
super(new UnnecessaryIgnoreAnalyzer());
}

@Test
void reportADiagnosticIfIgnoreIsUnnecessaryInIfs()
{
testDiagnostics(
"""
DEFINE DATA LOCAL
END-DEFINE
IF TRUE
WRITE 'Hi'
IGNORE
END-IF
END
""",
expectDiagnostic(4, UnnecessaryIgnoreAnalyzer.UNNECESSARY_IGNORE)
);
}

@Test
void reportNoDiagnosticIfIgnoreIsNeccessaryInIfs()
{
testDiagnostics(
"""
DEFINE DATA LOCAL
END-DEFINE
IF TRUE
IGNORE
END-IF
END
""",
expectNoDiagnosticOfType(UnnecessaryIgnoreAnalyzer.UNNECESSARY_IGNORE)
);
}

@Test
void reportADiagnosticIfIgnoreIsTheOnlyStatementInAModule()
{
testDiagnostics(
"""
DEFINE DATA LOCAL
END-DEFINE
IGNORE
END
""",
expectDiagnostic(2, UnnecessaryIgnoreAnalyzer.UNNECESSARY_IGNORE)
);
}

@Test
void reportNoDiagnosticIfIgnoreIsNeccessaryInDecideBlcosk()
{
testDiagnostics(
"""
DEFINE DATA LOCAL
1 #VAR (N1)
END-DEFINE
DECIDE ON FIRST VALUE OF #VAR
VALUE 1
IGNORE
VALUE 2
IGNORE
NONE VALUE
IGNORE
ANY VALUE
IGNORE
END-DECIDE
END
""",
expectNoDiagnosticOfType(UnnecessaryIgnoreAnalyzer.UNNECESSARY_IGNORE)
);
}
}
6 changes: 6 additions & 0 deletions tools/ruletranslator/src/main/resources/rules/NL025
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
name: IGNORE is unnecessary
priority: MINOR
tags: pitfall,confusing
type: CODE_SMELL
description:
If an ``IGNORE`` is not the only statement in a block, it is unnecessary and can be removed.