This is an automated email from the ASF dual-hosted git repository.
bamaer pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/hop.git
The following commit(s) were added to refs/heads/main by this push:
new 44d4d178d2 Fixes #8731 : Fix hop lint version, quiet, severity and
stdout output, warn about unknown rule ids and make the pre-commit hook find
staged files (#8739)
44d4d178d2 is described below
commit 44d4d178d27355210af97855099e1a7ad909b947
Author: Bart Maertens <[email protected]>
AuthorDate: Sat Oct 3 09:22:06 2026 +0200
Fixes #8731 : Fix hop lint version, quiet, severity and stdout output, warn
about unknown rule ids and make the pre-commit hook find staged files (#8739)
---
.../modules/ROOT/pages/hop-tools/hop-lint.adoc | 4 +-
.../ROOT/pages/linting/lint-rule-packs.adoc | 3 +
.../main/java/org/apache/hop/lint/LintCommand.java | 132 +++++++++---
.../java/org/apache/hop/lint/LintReportWriter.java | 23 +-
.../org/apache/hop/lint/PreCommitLintService.java | 24 ++-
.../apache/hop/lint/registry/EffectiveRuleSet.java | 12 ++
.../org/apache/hop/lint/registry/RuleRegistry.java | 92 +++++++-
.../java/org/apache/hop/lint/LintCommandTest.java | 232 +++++++++++++++++++++
.../apache/hop/lint/registry/RuleRegistryTest.java | 90 ++++++++
9 files changed, 567 insertions(+), 45 deletions(-)
diff --git a/docs/hop-user-manual/modules/ROOT/pages/hop-tools/hop-lint.adoc
b/docs/hop-user-manual/modules/ROOT/pages/hop-tools/hop-lint.adoc
index 57fe1bd19c..c1f121a34b 100644
--- a/docs/hop-user-manual/modules/ROOT/pages/hop-tools/hop-lint.adoc
+++ b/docs/hop-user-manual/modules/ROOT/pages/hop-tools/hop-lint.adoc
@@ -115,7 +115,8 @@ hop lint --max-warnings 20 .
`sarif` is the format GitHub code scanning, Azure DevOps and most review
tooling read, so findings show up as annotations on the pull request.
`json` is there for everything else.
-Both formats write only the report to stdout, so they can be piped.
+Stdout carries only the report, in every format, so it can be piped or
redirected.
+Hop's log lines and notes such as the baseline count go to stderr.
[[baseline]]
== Adopting the linter on an existing project
@@ -146,6 +147,7 @@ hop lint --install-hook
This writes a `pre-commit` hook into the current repository's `.git/hooks`.
The hook lints the staged Hop files and blocks the commit when they fail.
+It is a copy, so run `hop lint --install-hook` again after upgrading Hop to
pick up changes to it.
Hop Gui can do the same for commits made from within Hop Gui. See
*Configuration -> Linter -> Block Git Commits*, which is *off by default*:
installing and enabling the linter does not on its own change how committing
behaves.
diff --git
a/docs/hop-user-manual/modules/ROOT/pages/linting/lint-rule-packs.adoc
b/docs/hop-user-manual/modules/ROOT/pages/linting/lint-rule-packs.adoc
index 08063e4863..acb597ff37 100644
--- a/docs/hop-user-manual/modules/ROOT/pages/linting/lint-rule-packs.adoc
+++ b/docs/hop-user-manual/modules/ROOT/pages/linting/lint-rule-packs.adoc
@@ -113,6 +113,9 @@ rules:
This keeps the pack upgradeable: the project's decisions live with the
project, and the pack can be replaced with a newer version without losing them.
+Rule ids are matched ignoring case.
+An id that no installed pack defines, such as a typo, is reported as a warning
in the Hop log and by `hop lint`, with the closest id as a suggestion, and the
override is ignored.
+
The rule manager in Hop Gui writes these overrides for you.
Editing a pack rule there never changes the pack: a change to its severity,
threshold or enabled state is written to `hop-lint.yml` as an override, and a
change to what the rule actually looks at is written there as a complete rule
which replaces the pack rule of that id.
Pack rules cannot be deleted, only switched off.
diff --git
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintCommand.java
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintCommand.java
index cb3e789505..356ca8ba96 100644
--- a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintCommand.java
+++ b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintCommand.java
@@ -36,9 +36,12 @@ import lombok.Getter;
import lombok.Setter;
import org.apache.hop.core.Const;
import org.apache.hop.core.HopEnvironment;
+import org.apache.hop.core.HopVersionProvider;
import org.apache.hop.core.encryption.Encr;
import org.apache.hop.core.exception.HopException;
import org.apache.hop.core.logging.DefaultLogLevel;
+import org.apache.hop.core.logging.HopLogStore;
+import org.apache.hop.core.logging.LogChannel;
import org.apache.hop.core.logging.LogLevel;
import org.apache.hop.core.plugins.ActionPluginType;
import org.apache.hop.core.plugins.IPlugin;
@@ -71,6 +74,7 @@ import picocli.CommandLine.Parameters;
@Command(
name = "lint",
mixinStandardHelpOptions = true,
+ versionProvider = HopVersionProvider.class,
description =
"Check pipelines, workflows and metadata against the lint rules. Exits
1 when a finding "
+ "reaches the --fail-on threshold (ERROR by default) or warnings
exceed "
@@ -115,8 +119,8 @@ public class LintCommand implements Callable<Integer>,
IHopCommand {
names = {"-s", "--severity"},
paramLabel = "<severity>",
description =
- "Report only findings at this severity: ${COMPLETION-CANDIDATES}.
Narrows the report "
- + "only; --fail-on decides the exit code.")
+ "Report only findings at this severity or above:
${COMPLETION-CANDIDATES}. Narrows the "
+ + "report only; --fail-on decides the exit code.")
private LintSeverity.Level severityFilter;
@Option(
@@ -199,6 +203,11 @@ public class LintCommand implements Callable<Integer>,
IHopCommand {
@SuppressWarnings("java:S4507")
@Override
public Integer call() {
+ // Hop's console logging writes to the stdout it saw at start-up, not to
System.out, so moving
+ // System.out does not move it. Stdout carries the report, which a CI job
redirects to a file or
+ // pipes to jq; everything else goes to stderr.
+ PrintStream logOut = HopLogStore.OriginalSystemOut;
+ HopLogStore.OriginalSystemOut = HopLogStore.OriginalSystemErr;
try {
return run();
} catch (Exception e) {
@@ -207,6 +216,8 @@ public class LintCommand implements Callable<Integer>,
IHopCommand {
e.printStackTrace();
}
return 1;
+ } finally {
+ HopLogStore.OriginalSystemOut = logOut;
}
}
@@ -286,23 +297,60 @@ public class LintCommand implements Callable<Integer>,
IHopCommand {
if (reportOwnsStdout) {
System.setOut(stdout);
}
- outputResults(filterForDisplay(results));
+ report(results, exitCode != 0);
return exitCode;
} finally {
System.setOut(stdout);
}
}
- /** Apply {@code --severity}, which narrows the report only. */
- private List<LintResult> filterForDisplay(List<LintResult> results) {
+ /**
+ * Apply {@code --severity}, which narrows the report only, to the severity
given and above:
+ * {@code -s WARNING} on a project with errors has to show the errors.
+ */
+ List<LintResult> filterForDisplay(List<LintResult> results) {
if (severityFilter == null) {
return results;
}
return results.stream()
- .filter(result -> severityFilter.name().equals(result.getSeverity()))
+ .filter(result -> atOrAbove(result.getSeverity(), severityFilter))
.collect(Collectors.toList());
}
+ /** A severity this build does not know is shown rather than hidden. */
+ private static boolean atOrAbove(String severity, LintSeverity.Level
minimum) {
+ for (LintSeverity.Level level : LintSeverity.Level.values()) {
+ if (level.name().equalsIgnoreCase(severity)) {
+ return level.ordinal() <= minimum.ordinal();
+ }
+ }
+ return true;
+ }
+
+ /**
+ * Print the report after {@code --severity}, and say what that left out,
the way the baseline
+ * does. Hiding findings without a word made a failing run read "No lint
issues found."
+ */
+ private void report(List<LintResult> results, boolean failing) {
+ List<LintResult> shown = filterForDisplay(results);
+ int hidden = results.size() - shown.size();
+ if (hidden > 0 && !quiet) {
+ System.err.println(hidden + " finding(s) below " + severityFilter + "
hidden by --severity.");
+ LintSeverity.FailOn threshold = LintSeverity.parseFailOn(failOn);
+ if (failing
+ && shown.stream()
+ .noneMatch(r ->
LintSeverity.meetsFailOnThreshold(r.getSeverity(), threshold))) {
+ System.err.println(
+ "Failing: the findings at --fail-on "
+ + failOn
+ + " are below --severity "
+ + severityFilter
+ + " and not shown.");
+ }
+ }
+ outputResults(shown, hidden);
+ }
+
/**
* Hide the findings a project has already accepted, so a run reports only
what is new.
*
@@ -497,10 +545,14 @@ public class LintCommand implements Callable<Integer>,
IHopCommand {
}
}
- /** Version from the jar manifest, so it cannot drift from the build the way
a literal does. */
+ /** The Hop version, as {@code hop --version} and {@code hop lint --version}
print it. */
private static String toolVersion() {
- String version = LintCommand.class.getPackage().getImplementationVersion();
- return version != null ? version : "development build";
+ String[] version = new HopVersionProvider().getVersion();
+ if (version.length > 0 && !Utils.isEmpty(version[0])) {
+ return version[0];
+ }
+ String lintVersion =
LintCommand.class.getPackage().getImplementationVersion();
+ return lintVersion != null ? lintVersion : "development build";
}
private int runPreCommit() throws Exception {
@@ -514,7 +566,8 @@ public class LintCommand implements Callable<Integer>,
IHopCommand {
}
initializeHopEnvironment();
- List<File> stagedFiles =
PreCommitLintService.readStagedFiles(stagedFileList);
+ List<File> stagedFiles =
+ PreCommitLintService.readStagedFiles(stagedFileList, new
File(userDirectory()));
if (stagedFiles.isEmpty()) {
if (!quiet) {
System.out.println("No staged Hop files to lint.");
@@ -559,7 +612,7 @@ public class LintCommand implements Callable<Integer>,
IHopCommand {
// on findings that were already there before the change.
List<LintResult> results = applyBaseline(result.getResults());
- outputResults(filterForDisplay(results));
+ report(results, shouldFail(results, LintSeverity.parseFailOn(failOn)));
if (shouldFail(results, LintSeverity.parseFailOn(failOn))) {
long blocking =
@@ -639,20 +692,18 @@ public class LintCommand implements Callable<Integer>,
IHopCommand {
}
/**
- * Keep Hop's engine logging off stdout when stdout is carrying a report.
+ * {@code --quiet} reports findings only, so Hop's progress logging goes
too. Warnings about the
+ * configuration, such as a rule id that does not exist, are logged at the
minimal level and stay.
*
- * <p>Hop logs to the console by default. That is fine for the text report,
but {@code -f sarif}
- * piped to another tool has to be a valid document, and interleaved log
lines make it garbage. An
- * explicit {@code --output} file keeps the two streams apart, so logging is
left alone there.
+ * <p>The general log channel exists before this command runs and read the
default level when it
+ * was created, so it has to be set on its own; setting the default alone
changed nothing.
*/
private void quietenHopLogging() {
- boolean reportOwnsStdout = format != LintReportFormat.TEXT && outputFile
== null;
- if (verbose) {
+ if (verbose || !quiet) {
return;
}
- if (quiet || reportOwnsStdout) {
- DefaultLogLevel.setLogLevel(LogLevel.NOTHING);
- }
+ DefaultLogLevel.setLogLevel(LogLevel.MINIMAL);
+ LogChannel.GENERAL.setLogLevel(LogLevel.MINIMAL);
}
private void loadConfiguration(HopLinter linter, String targetPath) throws
IOException {
@@ -793,14 +844,21 @@ public class LintCommand implements Callable<Integer>,
IHopCommand {
* working when Hop is upgraded or moved, and so the same hook can be
committed to a repository
* that several people clone.
*/
- private String hookScript() {
+ String hookScript() {
+ // git lists staged files relative to the repository root, and the hop
launcher changes to the
+ // Hop installation before it starts Java, so the paths are made absolute
here. The trap
+ // removes the list however the script ends; the last command's status is
the hook's.
return """
#!/bin/sh
- set -e
- STAGED_LIST="$(mktemp)"
- git diff --cached --name-only --diff-filter=ACM > "$STAGED_LIST"
- if ! grep -E '\\.(hpl|hwf)$|/metadata/.*\\.json$' "$STAGED_LIST" >
/dev/null 2>&1; then
- rm -f "$STAGED_LIST"
+ ROOT="$(git rev-parse --show-toplevel)" || exit 1
+ STAGED_LIST="$(mktemp)" || exit 1
+ trap 'rm -f "$STAGED_LIST"' EXIT
+ git diff --cached --name-only --diff-filter=ACM | while IFS= read -r
FILE; do
+ case "$FILE" in
+ *.hpl|*.hwf|metadata/*.json|*/metadata/*.json) printf '%s/%s\\n'
"$ROOT" "$FILE" ;;
+ esac
+ done > "$STAGED_LIST"
+ if [ ! -s "$STAGED_LIST" ]; then
exit 0
fi
HOP="${HOP_HOME:-}/hop"
@@ -809,24 +867,30 @@ public class LintCommand implements Callable<Integer>,
IHopCommand {
fi
if [ -z "$HOP" ] || [ ! -x "$HOP" ]; then
echo "The hop launcher was not found. Set HOP_HOME, or put hop on
the PATH." >&2
- rm -f "$STAGED_LIST"
exit 1
fi
- "$HOP" lint --pre-commit --staged-file "$STAGED_LIST" \
+ "$HOP" lint --pre-commit --staged-file "$STAGED_LIST" \\
${HOP_LINT_FAIL_ON:+--fail-on "$HOP_LINT_FAIL_ON"}
- STATUS=$?
- rm -f "$STAGED_LIST"
- exit $STATUS
""";
}
- private void outputResults(List<LintResult> results) {
+ /**
+ * @param hidden how many findings {@code --severity} left out, so an empty
text report does not
+ * claim there were none
+ */
+ private void outputResults(List<LintResult> results, int hidden) {
String report;
try {
- report = LintReportWriter.render(format, results, toolVersion(),
reportBaseDirectory());
+ if (format == LintReportFormat.TEXT && results.isEmpty() && hidden > 0) {
+ report = "No findings at " + severityFilter + " or above.\n";
+ } else if (format == LintReportFormat.TEXT) {
+ report = LintReportWriter.renderText(results, !quiet);
+ } else {
+ report = LintReportWriter.render(format, results, toolVersion(),
reportBaseDirectory());
+ }
} catch (Exception e) {
System.err.println("Error rendering the " + format.getId() + " report: "
+ e.getMessage());
- report = LintReportWriter.renderText(results);
+ report = LintReportWriter.renderText(results, !quiet);
}
if (outputFile != null) {
diff --git
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintReportWriter.java
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintReportWriter.java
index 5217cd35d2..d86e3c1b9d 100644
--- a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintReportWriter.java
+++ b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintReportWriter.java
@@ -52,24 +52,33 @@ public final class LintReportWriter {
return renderSarif(results, toolVersion, baseDirectory);
case TEXT:
default:
- return renderText(results);
+ return renderText(results, true);
}
}
// ---------------------------------------------------------------- text
public static String renderText(List<LintResult> results) {
+ return renderText(results, true);
+ }
+
+ /**
+ * @param summary whether to open with the totals; {@code --quiet} reports
the findings only
+ */
+ public static String renderText(List<LintResult> results, boolean summary) {
StringBuilder out = new StringBuilder();
if (results.isEmpty()) {
return "No lint issues found.\n";
}
- out.append("Lint Results Summary:\n");
- out.append("===================\n");
- out.append("Total Issues: ").append(results.size()).append('\n');
- out.append("Errors: ").append(countBySeverity(results,
"ERROR")).append('\n');
- out.append("Warnings: ").append(countBySeverity(results,
"WARNING")).append('\n');
- out.append("Info: ").append(countBySeverity(results,
"INFO")).append("\n\n");
+ if (summary) {
+ out.append("Lint Results Summary:\n");
+ out.append("===================\n");
+ out.append("Total Issues: ").append(results.size()).append('\n');
+ out.append("Errors: ").append(countBySeverity(results,
"ERROR")).append('\n');
+ out.append("Warnings: ").append(countBySeverity(results,
"WARNING")).append('\n');
+ out.append("Info: ").append(countBySeverity(results,
"INFO")).append("\n\n");
+ }
for (String severity : new String[] {"ERROR", "WARNING", "INFO"}) {
List<LintResult> group =
diff --git
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/PreCommitLintService.java
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/PreCommitLintService.java
index 77954090f7..8b1cce6720 100644
---
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/PreCommitLintService.java
+++
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/PreCommitLintService.java
@@ -93,6 +93,19 @@ public final class PreCommitLintService {
}
public static List<File> readStagedFiles(String stagedFileListPath) throws
HopException {
+ return readStagedFiles(stagedFileListPath, null);
+ }
+
+ /**
+ * The staged files to lint.
+ *
+ * @param baseDirectory what a relative path in the list is relative to. git
lists staged files
+ * relative to the repository root, and the hop launcher changes to the
Hop installation
+ * before it starts Java, so resolving them against the working
directory found none of them
+ * and the hook let every commit through.
+ */
+ public static List<File> readStagedFiles(String stagedFileListPath, File
baseDirectory)
+ throws HopException {
List<File> files = new ArrayList<>();
File listFile = new File(stagedFileListPath);
if (!listFile.isFile()) {
@@ -106,8 +119,17 @@ public final class PreCommitLintService {
continue;
}
File candidate = new File(trimmed);
- if (candidate.isFile() && isLintablePath(trimmed)) {
+ if (!candidate.isAbsolute() && baseDirectory != null) {
+ candidate = new File(baseDirectory, trimmed);
+ }
+ if (!isLintablePath(candidate.getAbsolutePath())) {
+ continue;
+ }
+ if (candidate.isFile()) {
files.add(candidate);
+ } else {
+ // A staged file the hook cannot find is a hook that checks nothing,
so say so.
+ System.err.println("Staged file not found, not linted: " +
candidate.getPath());
}
}
} catch (Exception e) {
diff --git
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/registry/EffectiveRuleSet.java
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/registry/EffectiveRuleSet.java
index 8f17447167..4bb9c5f4b1 100644
---
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/registry/EffectiveRuleSet.java
+++
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/registry/EffectiveRuleSet.java
@@ -29,12 +29,19 @@ public final class EffectiveRuleSet {
private final List<CustomLintRule> rules;
private final LinterConfig config;
private final LintPolicy policy;
+ private final List<String> warnings;
public EffectiveRuleSet(List<CustomLintRule> rules, LinterConfig config) {
this(rules, config, LintPolicy.empty());
}
public EffectiveRuleSet(List<CustomLintRule> rules, LinterConfig config,
LintPolicy policy) {
+ this(rules, config, policy, List.of());
+ }
+
+ public EffectiveRuleSet(
+ List<CustomLintRule> rules, LinterConfig config, LintPolicy policy,
List<String> warnings) {
+ this.warnings = warnings != null ? List.copyOf(warnings) : List.of();
this.rules =
rules != null
? Collections.unmodifiableList(new ArrayList<>(rules))
@@ -43,6 +50,11 @@ public final class EffectiveRuleSet {
this.policy = policy != null ? policy : LintPolicy.empty();
}
+ /** Problems with the project's hop-lint.yml that did not stop it loading,
such as unknown ids. */
+ public List<String> getWarnings() {
+ return warnings;
+ }
+
/** What the project excludes from linting, and which findings it has
accepted. */
public LintPolicy getPolicy() {
return policy;
diff --git
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/registry/RuleRegistry.java
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/registry/RuleRegistry.java
index c838161e95..746aaca0f7 100644
---
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/registry/RuleRegistry.java
+++
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/registry/RuleRegistry.java
@@ -18,8 +18,12 @@ package org.apache.hop.lint.registry;
import java.io.File;
import java.util.ArrayList;
+import java.util.Collection;
import java.util.LinkedHashMap;
+import java.util.List;
import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
import org.apache.hop.core.logging.LogChannel;
import org.apache.hop.core.util.Utils;
import org.apache.hop.lint.CustomLintRule;
@@ -47,6 +51,13 @@ public class RuleRegistry {
*/
private volatile Map<String, CustomLintRule> packRules;
+ /**
+ * The unknown rule id warnings already logged, per hop-lint.yml. Resolution
runs once per file
+ * and on every background check, so logging each time repeated the same
line for as long as the
+ * project kept the id.
+ */
+ private final Set<String> loggedWarnings = ConcurrentHashMap.newKeySet();
+
public static RuleRegistry getInstance() {
return INSTANCE;
}
@@ -138,6 +149,7 @@ public class RuleRegistry {
}
ProjectYamlOverlay overlay = ProjectYamlOverlay.empty();
+ List<String> warnings = new ArrayList<>();
if (projectYaml != null && projectYaml.exists()) {
try {
overlay = YamlRulePackParser.parseProjectYaml(projectYaml);
@@ -146,9 +158,19 @@ public class RuleRegistry {
}
for (Map.Entry<String, ProjectYamlOverlay.ProjectRuleOverlay> entry :
overlay.getOverlays().entrySet()) {
- CustomLintRule existing = merged.get(entry.getKey());
+ CustomLintRule existing = findRule(merged, entry.getKey());
if (existing != null) {
entry.getValue().applyTo(existing);
+ } else {
+ // Applied to nothing, a typo such as SQL-02 for SQL-002 left the
rule as it was with
+ // no sign anything had gone wrong.
+ String warning = unknownRuleWarning(entry.getKey(),
merged.keySet(), projectYaml);
+ warnings.add(warning);
+ if (loggedWarnings.add(projectYaml.getAbsolutePath() + '\n' +
warning)) {
+ LogChannel.GENERAL.logMinimal(warning);
+ } else {
+ LogChannel.GENERAL.logDetailed(warning);
+ }
}
}
LogChannel.GENERAL.logDetailed(
@@ -168,7 +190,73 @@ public class RuleRegistry {
LinterConfig config = buildLinterConfig(merged);
config.setEnabled(true);
- return new EffectiveRuleSet(new ArrayList<>(merged.values()), config,
overlay.getPolicy());
+ return new EffectiveRuleSet(
+ new ArrayList<>(merged.values()), config, overlay.getPolicy(),
warnings);
+ }
+
+ /** The rule of this id, ignoring case: {@code sql-002} in hop-lint.yml
means SQL-002. */
+ private static CustomLintRule findRule(Map<String, CustomLintRule> rules,
String ruleId) {
+ CustomLintRule exact = rules.get(ruleId);
+ if (exact != null) {
+ return exact;
+ }
+ for (Map.Entry<String, CustomLintRule> entry : rules.entrySet()) {
+ if (entry.getKey().equalsIgnoreCase(ruleId)) {
+ return entry.getValue();
+ }
+ }
+ return null;
+ }
+
+ /**
+ * A warning, not an error: a project may tune a rule from a pack that is
not installed on every
+ * machine that lints it, and that should not stop the run.
+ */
+ static String unknownRuleWarning(String ruleId, Collection<String> knownIds,
File projectYaml) {
+ StringBuilder warning =
+ new StringBuilder("Warning: ")
+ .append(projectYaml.getName())
+ .append(" changes rule '")
+ .append(ruleId)
+ .append("', which no installed rule pack defines; the change is
ignored.");
+ String closest = closestId(ruleId, knownIds);
+ if (closest != null) {
+ warning.append(" Did you mean ").append(closest).append("?");
+ }
+ return warning.toString();
+ }
+
+ /** The known id within two edits of the one given, or null. */
+ private static String closestId(String ruleId, Collection<String> knownIds) {
+ String best = null;
+ int bestDistance = 3;
+ for (String known : knownIds) {
+ int distance = editDistance(ruleId.toUpperCase(), known.toUpperCase());
+ if (distance < bestDistance) {
+ bestDistance = distance;
+ best = known;
+ }
+ }
+ return best;
+ }
+
+ private static int editDistance(String a, String b) {
+ int[] previous = new int[b.length() + 1];
+ int[] current = new int[b.length() + 1];
+ for (int j = 0; j <= b.length(); j++) {
+ previous[j] = j;
+ }
+ for (int i = 1; i <= a.length(); i++) {
+ current[0] = i;
+ for (int j = 1; j <= b.length(); j++) {
+ int substitution = previous[j - 1] + (a.charAt(i - 1) == b.charAt(j -
1) ? 0 : 1);
+ current[j] = Math.min(substitution, Math.min(previous[j] + 1,
current[j - 1] + 1));
+ }
+ int[] swap = previous;
+ previous = current;
+ current = swap;
+ }
+ return previous[b.length()];
}
public EffectiveRuleSet resolveForContext(File context) {
diff --git
a/plugins/misc/lint/src/test/java/org/apache/hop/lint/LintCommandTest.java
b/plugins/misc/lint/src/test/java/org/apache/hop/lint/LintCommandTest.java
new file mode 100644
index 0000000000..9d0a63c3a1
--- /dev/null
+++ b/plugins/misc/lint/src/test/java/org/apache/hop/lint/LintCommandTest.java
@@ -0,0 +1,232 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hop.lint;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assumptions.assumeTrue;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.PrintStream;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.List;
+import java.util.Map;
+import org.apache.hop.core.HopVersionProvider;
+import org.apache.hop.core.logging.HopLogStore;
+import org.apache.hop.core.logging.LogChannel;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+import picocli.CommandLine;
+
+/**
+ * The {@code hop lint} command line.
+ *
+ * @see <a href="https://github.com/apache/hop/issues/8731";>#8731</a>
+ */
+public class LintCommandTest {
+
+ @TempDir private Path dir;
+
+ private static LintResult finding(String ruleId, String severity) {
+ return new LintResult(ruleId, ruleId, severity, "message",
"/tmp/project/a.hpl");
+ }
+
+ @Test
+ public void versionComesFromHop() {
+ CommandLine commandLine = new CommandLine(new LintCommand());
+
+ assertInstanceOf(
+ HopVersionProvider.class,
+ commandLine.getCommandSpec().versionProvider(),
+ "-V printed " + "nothing without a version provider");
+ }
+
+ @Test
+ public void severityIsAMinimum() {
+ LintCommand command = new LintCommand();
+ command.setSeverityFilter(LintSeverity.Level.WARNING);
+
+ List<LintResult> shown =
+ command.filterForDisplay(
+ List.of(finding("A-1", "ERROR"), finding("B-1", "WARNING"),
finding("C-1", "INFO")));
+
+ assertEquals(
+ List.of("A-1", "B-1"),
shown.stream().map(LintResult::getRuleId).toList(), "-s WARNING");
+ }
+
+ @Test
+ public void quietLeavesOutTheSummary() {
+ List<LintResult> results = List.of(finding("A-1", "ERROR"));
+
+ String quiet = LintReportWriter.renderText(results, false);
+
+ assertFalse(quiet.contains("Lint Results Summary"), quiet);
+ assertTrue(quiet.contains("A-1"), quiet);
+ assertTrue(LintReportWriter.renderText(results).contains("Lint Results
Summary"));
+ }
+
+ /**
+ * Hop logs to the stdout it saw at start-up, so piping a JSON report to jq
got log lines in front
+ * of the document.
+ */
+ @Test
+ public void aJsonReportIsAloneOnStdout() throws Exception {
+ Files.createDirectories(dir.resolve("project"));
+ ByteArrayOutputStream captured = new ByteArrayOutputStream();
+ PrintStream capture = new PrintStream(captured, true,
StandardCharsets.UTF_8);
+
+ PrintStream systemOut = System.out;
+ PrintStream logOut = HopLogStore.OriginalSystemOut;
+ int exitCode;
+ try {
+ System.setOut(capture);
+ HopLogStore.OriginalSystemOut = capture;
+ LogChannel.GENERAL.logBasic("A log line that must not reach stdout");
+ captured.reset();
+
+ exitCode =
+ new CommandLine(new LintCommand())
+ .execute("-f", "JSON", dir.resolve("project").toString());
+ } finally {
+ System.setOut(systemOut);
+ HopLogStore.OriginalSystemOut = logOut;
+ }
+
+ String stdout = captured.toString(StandardCharsets.UTF_8);
+ assertEquals(0, exitCode, stdout);
+ JsonNode report = new ObjectMapper().readTree(stdout);
+ assertEquals(0, report.get("summary").get("total").asInt(), stdout);
+ assertTrue(stdout.trim().startsWith("{"), stdout);
+ }
+
+ // ------------------------------------------------------------------
pre-commit hook
+
+ private static boolean canRunShell() {
+ return !System.getProperty("os.name").toLowerCase().contains("win")
+ && new File("/bin/sh").canExecute();
+ }
+
+ private static int run(Path workDir, Map<String, String> env, String...
command)
+ throws Exception {
+ ProcessBuilder builder = new
ProcessBuilder(command).directory(workDir.toFile());
+ builder.environment().putAll(env);
+ builder.redirectErrorStream(true);
+ Process process = builder.start();
+ process.getInputStream().readAllBytes();
+ return process.waitFor();
+ }
+
+ /** A repository with staged files, and a stand-in launcher that records
what it was given. */
+ private Path repositoryWithStagedFiles(String... files) throws Exception {
+ Path repo = dir.resolve("repo");
+ Files.createDirectories(repo);
+ assumeTrue(run(repo, Map.of(), "git", "init", "-q") == 0, "git is needed
for this test");
+ for (String file : files) {
+ Path path = repo.resolve(file);
+ Files.createDirectories(path.getParent());
+ Files.writeString(path, "content");
+ }
+ assertEquals(0, run(repo, Map.of(), "git", "add", "."));
+
+ Path hopHome = dir.resolve("hop-home");
+ Files.createDirectories(hopHome);
+ Path launcher = hopHome.resolve("hop");
+ Files.writeString(
+ launcher,
+ """
+ #!/bin/sh
+ while [ $# -gt 0 ]; do
+ if [ "$1" = "--staged-file" ]; then
+ cat "$2" > "$CAPTURE"
+ echo "$2" > "$CAPTURE.path"
+ fi
+ shift
+ done
+ exit 1
+ """);
+ assertTrue(launcher.toFile().setExecutable(true));
+
+ Files.writeString(dir.resolve("pre-commit"), new
LintCommand().hookScript());
+ return repo;
+ }
+
+ private Map<String, String> hookEnvironment() {
+ return Map.of(
+ "HOP_HOME", dir.resolve("hop-home").toString(),
+ "CAPTURE", dir.resolve("capture").toString());
+ }
+
+ /**
+ * git lists staged files relative to the repository root, and the launcher
changes to the Hop
+ * installation, so none were found and every commit passed. Metadata at the
root of the
+ * repository was skipped too: "metadata/rdbms/x.json" has no leading slash.
+ */
+ @Test
+ public void theHookPassesAbsolutePathsAndBlocksTheCommit() throws Exception {
+ assumeTrue(canRunShell());
+ Path repo =
+ repositoryWithStagedFiles("load/customers.hpl",
"metadata/rdbms/crm.json", "notes.txt");
+
+ int status = run(repo, hookEnvironment(), "/bin/sh",
dir.resolve("pre-commit").toString());
+
+ assertEquals(1, status, "a failing lint has to block the commit");
+ String root = repo.toRealPath().toString();
+ List<String> staged = Files.readAllLines(dir.resolve("capture"));
+ assertEquals(
+ List.of(root + "/load/customers.hpl", root +
"/metadata/rdbms/crm.json"),
+ staged.stream().map(line ->
Path.of(line).toString()).sorted().toList());
+
+ String listFile = Files.readString(dir.resolve("capture.path")).trim();
+ assertFalse(Files.exists(Path.of(listFile)), "the staged list is left
behind: " + listFile);
+ }
+
+ @Test
+ public void theHookDoesNotStartHopWithoutHopFiles() throws Exception {
+ assumeTrue(canRunShell());
+ Path repo = repositoryWithStagedFiles("notes.txt");
+
+ int status = run(repo, hookEnvironment(), "/bin/sh",
dir.resolve("pre-commit").toString());
+
+ assertEquals(0, status);
+ assertFalse(Files.exists(dir.resolve("capture")), "hop was started for a
text file");
+ }
+
+ @Test
+ public void stagedPathsAreResolvedAgainstTheRepository() throws Exception {
+ Path repo = dir.resolve("repo");
+ Files.createDirectories(repo.resolve("metadata/rdbms"));
+ Files.writeString(repo.resolve("customers.hpl"), "content");
+ Files.writeString(repo.resolve("metadata/rdbms/crm.json"), "{}");
+ Path list = dir.resolve("staged.txt");
+ Files.writeString(list,
"customers.hpl\nmetadata/rdbms/crm.json\nmissing.hpl\nnotes.txt\n");
+
+ List<File> files = PreCommitLintService.readStagedFiles(list.toString(),
repo.toFile());
+
+ assertEquals(
+ List.of(
+ repo.resolve("customers.hpl").toFile(),
+ repo.resolve("metadata/rdbms/crm.json").toFile()),
+ files);
+ }
+}
diff --git
a/plugins/misc/lint/src/test/java/org/apache/hop/lint/registry/RuleRegistryTest.java
b/plugins/misc/lint/src/test/java/org/apache/hop/lint/registry/RuleRegistryTest.java
index 2475f5dd5a..97c99dabe0 100644
---
a/plugins/misc/lint/src/test/java/org/apache/hop/lint/registry/RuleRegistryTest.java
+++
b/plugins/misc/lint/src/test/java/org/apache/hop/lint/registry/RuleRegistryTest.java
@@ -22,8 +22,12 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
import java.io.File;
import java.nio.file.Files;
+import java.util.ArrayList;
import java.util.List;
import java.util.Map;
+import org.apache.hop.core.logging.HopLogStore;
+import org.apache.hop.core.logging.IHopLoggingEventListener;
+import org.apache.hop.core.logging.LogLevel;
import org.apache.hop.lint.CustomLintRule;
import org.apache.hop.lint.RuleCombinator;
import org.junit.jupiter.api.Test;
@@ -432,4 +436,90 @@ public class RuleRegistryTest {
projectYaml.delete();
}
}
+
+ /**
+ * A typo in hop-lint.yml used to change nothing and say nothing: SQL-002
stayed disabled.
+ *
+ * @see <a href="https://github.com/apache/hop/issues/8731";>#8731</a>
+ */
+ @Test
+ public void anUnknownRuleIdIsReportedWithTheClosestId() throws Exception {
+ File projectYaml = File.createTempFile("hop-lint", ".yml");
+ Files.writeString(projectYaml.toPath(), "rules:\n SQL-02:\n enabled:
true\n");
+ try {
+ EffectiveRuleSet rules = RuleRegistry.getInstance().resolve(projectYaml);
+
+ assertEquals(1, rules.getWarnings().size(),
rules.getWarnings().toString());
+ String warning = rules.getWarnings().get(0);
+ assertTrue(warning.contains("'SQL-02'"), warning);
+ assertTrue(warning.contains("Did you mean SQL-002?"), warning);
+ assertFalse(
+ rules.getRules().stream()
+ .filter(rule -> "SQL-002".equals(rule.generateRuleId()))
+ .findFirst()
+ .orElseThrow()
+ .isEnabled());
+ } finally {
+ Files.deleteIfExists(projectYaml.toPath());
+ }
+ }
+
+ @Test
+ public void aRuleIdInAnotherCaseIsTheSameRule() throws Exception {
+ File projectYaml = File.createTempFile("hop-lint", ".yml");
+ Files.writeString(projectYaml.toPath(), "rules:\n trans-002:\n
enabled: false\n");
+ try {
+ EffectiveRuleSet rules = RuleRegistry.getInstance().resolve(projectYaml);
+
+ assertTrue(rules.getWarnings().isEmpty(),
rules.getWarnings().toString());
+ assertFalse(
+ rules.getRules().stream()
+ .filter(rule -> "TRANS-002".equals(rule.generateRuleId()))
+ .findFirst()
+ .orElseThrow()
+ .isEnabled());
+ } finally {
+ Files.deleteIfExists(projectYaml.toPath());
+ }
+ }
+
+ /**
+ * Hop Gui resolves the rules for every file it lints and on every
background check, so a warning
+ * logged each time filled the log for as long as the project kept the id.
+ */
+ @Test
+ public void anUnknownRuleIdIsLoggedOnce() throws Exception {
+ HopLogStore.init();
+ File projectYaml = File.createTempFile("hop-lint", ".yml");
+ Files.writeString(projectYaml.toPath(), "rules:\n NO-SUCH-RULE:\n
enabled: true\n");
+ List<String> logged = new ArrayList<>();
+ IHopLoggingEventListener listener =
+ event -> {
+ if (event.getLevel() == LogLevel.MINIMAL
+ &&
String.valueOf(event.getMessage()).contains("'NO-SUCH-RULE'")) {
+ logged.add(String.valueOf(event.getMessage()));
+ }
+ };
+ HopLogStore.getAppender().addLoggingEventListener(listener);
+ try {
+ EffectiveRuleSet first = RuleRegistry.getInstance().resolve(projectYaml);
+ EffectiveRuleSet second =
RuleRegistry.getInstance().resolve(projectYaml);
+
+ assertEquals(1, logged.size(), logged.toString());
+ assertEquals(1, first.getWarnings().size());
+ assertEquals(first.getWarnings(), second.getWarnings(), "every
resolution still reports it");
+ } finally {
+ HopLogStore.getAppender().removeLoggingEventListener(listener);
+ Files.deleteIfExists(projectYaml.toPath());
+ }
+ }
+
+ @Test
+ public void anIdFarFromAnyRuleGetsNoSuggestion() throws Exception {
+ String warning =
+ RuleRegistry.unknownRuleWarning(
+ "COMPLETELY-DIFFERENT", List.of("SQL-002", "DB-001"), new
File("hop-lint.yml"));
+
+ assertFalse(warning.contains("Did you mean"), warning);
+ }
}
