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

Added hidden work file analyzer #523

Merged
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
1 change: 1 addition & 0 deletions docs/analyzer-config.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ The following configurations can be set in a `.editorconfig` file to configure p
| `natls.style.discourage_gitmarkers` | `true`, `false` | [`NL030`](../tools/ruletranslator/src/main/resources/rules/NL030)|
| `natls.style.discourage_inlineparameters` | `true`, `false` | [`NL031`](../tools/ruletranslator/src/main/resources/rules/NL031)|
| `natls.style.discourage_hiddentransactions` | `true`, `false` | [`NL032`](../tools/ruletranslator/src/main/resources/rules/NL032)|
| `natls.style.discourage_hiddenworkfiles` | `true`, `false` | [`NL033`](../tools/ruletranslator/src/main/resources/rules/NL033)|

# Example

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
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.ISyntaxNode;
import org.amshove.natparse.natural.project.NaturalFileType;
import org.amshove.natparse.natural.IReadWorkNode;
import org.amshove.natparse.natural.IWriteWorkNode;
import org.amshove.natparse.natural.ICloseWorkNode;
import org.amshove.natparse.natural.IDefineWorkFileNode;

public class HiddenWorkfileAnalyzer extends AbstractAnalyzer
{
public static final DiagnosticDescription HIDDEN_WORKFILE_STATEMENT_IS_DISCOURAGED = DiagnosticDescription.create(
"NL033",
"Use of [DEFINE|READ|WRITE|CLOSE] WORK FILE statement outside Program is discouraged.",
DiagnosticSeverity.WARNING
);

private boolean isWorkfileAnalyzerOff;

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

@Override
public void initialize(ILinterContext context)
{
context.registerNodeAnalyzer(IDefineWorkFileNode.class, this::analyzeTransaction);
context.registerNodeAnalyzer(IReadWorkNode.class, this::analyzeTransaction);
context.registerNodeAnalyzer(IWriteWorkNode.class, this::analyzeTransaction);
context.registerNodeAnalyzer(ICloseWorkNode.class, this::analyzeTransaction);
}

@Override
public void beforeAnalyzing(IAnalyzeContext context)
{
isWorkfileAnalyzerOff = !context.getConfiguration(context.getModule().file(), "natls.style.discourage_hiddenworkfiles", OPTION_FALSE).equalsIgnoreCase(OPTION_TRUE);
}

private void analyzeTransaction(ISyntaxNode node, IAnalyzeContext context)
{
if (isWorkfileAnalyzerOff)
{
return;
}

if (context.getModule().file().getFiletype() == NaturalFileType.PROGRAM)
{
return;
}

if (!NodeUtil.moduleContainsNode(context.getModule(), node))
{
return;
}

context.report(HIDDEN_WORKFILE_STATEMENT_IS_DISCOURAGED.createDiagnostic(node));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import org.amshove.natlint.linter.AbstractAnalyzerTest;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.provider.CsvSource;
import org.junit.jupiter.params.provider.ValueSource;

Expand Down Expand Up @@ -73,4 +74,22 @@ void notRaiseADiagnosticForETInProgram(String statement)
expectNoDiagnostic(0, HiddenTransactionAnalyzer.HIDDEN_TRANSACTION_STATEMENT_IS_DISCOURAGED)
);
}

@Test
void raiseNoDiagnosticIfOptionIsFalse()
{
configureEditorConfig("""
[*]
natls.style.discourage_hiddentransactions=false
""");

testDiagnostics(
"OBJECT.NSN", """
END TRANSACTION
END
""",
expectNoDiagnosticOfType(HiddenTransactionAnalyzer.HIDDEN_TRANSACTION_STATEMENT_IS_DISCOURAGED)
);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
package org.amshove.natlint.analyzers;

import org.amshove.natlint.linter.AbstractAnalyzerTest;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.provider.CsvSource;
import org.junit.jupiter.params.provider.ValueSource;

class HiddenWorkfileAnalyzerShould extends AbstractAnalyzerTest
{
protected HiddenWorkfileAnalyzerShould()
{
super(new HiddenWorkfileAnalyzer());
}

@ParameterizedTest
@CsvSource(
{
"C,DEFINE WORK FILE 1",
"C,READ WORK FILE 1 #RECORD;END-WORK",
"C,WRITE WORK FILE 1 #RECORD",
"C,CLOSE WORK FILE 1 TRANSACTION",
"H,DEFINE WORK FILE 1",
"H,READ WORK FILE 1 #RECORD;END-WORK",
"H,WRITE WORK FILE 1 #RECORD",
"H,CLOSE WORK FILE 1",
"M,DEFINE WORK FILE 1",
"M,READ WORK FILE 1 #RECORD;END-WORK",
"M,WRITE WORK FILE 1 #RECORD",
"M,CLOSE WORK FILE 1",
"N,DEFINE WORK FILE 1",
"N,READ WORK FILE 1 ONCE #RECORD",
"N,WRITE WORK FILE 1 #RECORD",
"N,CLOSE WORK FILE 1",
"S,DEFINE WORK FILE 1",
"S,READ WORK FILE 1 #RECORD;END-WORK",
"S,WRITE WORK FILE 1 #RECORD",
"S,CLOSE WORK FILE 1"
}
)
void raiseADiagnosticForETForOtherObjectTypesThanProgram(String objtype, String statement)
{
configureEditorConfig("""
[*]
natls.style.discourage_hiddenworkfiles=true
""");

testDiagnostics(
"OBJECT.NS%s".formatted(objtype), """
%s
END
""".formatted(statement),
expectDiagnostic(0, HiddenWorkfileAnalyzer.HIDDEN_WORKFILE_STATEMENT_IS_DISCOURAGED)
);
}

@ParameterizedTest
@ValueSource(strings =
{
"DEFINE WORK FILE 1",
"READ WORK FILE 1 #RECORD;END-WORK",
"READ WORK FILE 1 ONCE #RECORD",
"WRITE WORK FILE 1 #RECORD",
"CLOSE WORK FILE 1",
"STOP"
})
void notRaiseADiagnosticForETInProgram(String statement)
{
configureEditorConfig("""
[*]
natls.style.discourage_hiddenworkfiles=true
""");

testDiagnostics(
"OBJECT.NSP", """
%s
END
""".formatted(statement),
expectNoDiagnostic(0, HiddenWorkfileAnalyzer.HIDDEN_WORKFILE_STATEMENT_IS_DISCOURAGED)
);
}

@Test
void raiseNoDiagnosticIfOptionIsFalse()
{
configureEditorConfig("""
[*]
natls.style.discourage_hiddenworkfiles=false
""");

testDiagnostics(
"OBJECT.NSN", """
CLOSE WORK FILE 1
END
""",
expectNoDiagnosticOfType(HiddenWorkfileAnalyzer.HIDDEN_WORKFILE_STATEMENT_IS_DISCOURAGED)
);
}
}
6 changes: 6 additions & 0 deletions tools/ruletranslator/src/main/resources/rules/NL033
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
name: Use of DEFINE/READ/WRITE/CLOSE WORK FILE statements outside of Natural Program is discouraged
priority: MAJOR
tags: clumsy, confusing, bad-practice, pitfall
type: CODE_SMELL
description:
Similar to transaction logic, work file handling should only be placed in Natural Programs, because this is the top layer of Natural objects. If you put them in other Natural objects, these can be called from other objects, and then you have little control over work file handling, anymore. However, there can be exceptions to this - so not as strongly discouraged as transaction logic.