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 a7e7a21b43 Fixes #8735 : Sort lint results, default the Ignore dialog
to the rep… (#8745)
a7e7a21b43 is described below
commit a7e7a21b43aad61999674de31c7b96bef2afb378
Author: Bart Maertens <[email protected]>
AuthorDate: Fri Oct 9 19:59:55 2026 +0200
Fixes #8735 : Sort lint results, default the Ignore dialog to the rep…
(#8745)
* Fixes #8735 : Sort lint results, default the Ignore dialog to the
reported rules, stop writing results files and mark ignored elements clearly
* Issue #8735 : Set the linter options' enabled state when the
configuration page opens, resolve each path once when sorting lint results
---
.../modules/ROOT/pages/hop-tools/hop-lint.adoc | 2 +-
.../modules/ROOT/pages/linting/index.adoc | 2 +-
.../modules/ROOT/pages/linting/lint-rules.adoc | 2 +-
.../modules/ROOT/partials/config/all-plugins.adoc | 34 ++--
.../ROOT/partials/config/linter-config.adoc | 34 ++--
.../apache/hop/lint/LintCanvasOverlayHelper.java | 19 ++-
.../org/apache/hop/lint/LintResultGrouping.java | 58 +++++++
.../java/org/apache/hop/lint/LintResultsPanel.java | 122 ++++++-------
.../org/apache/hop/lint/LintSuppressDialog.java | 24 +--
.../org/apache/hop/lint/LinterConfigPlugin.java | 27 ++-
.../java/org/apache/hop/lint/LinterGuiPlugin.java | 188 ++-------------------
.../hop/lint/messages/messages_en_US.properties | 8 +-
.../org/apache/hop/lint/LintResultsViewTest.java | 131 ++++++++++++++
.../hop/lint/LintSuppressDialogDefaultTest.java | 62 +++++++
.../configuration/tabs/ConfigPluginOptionsTab.java | 21 ++-
.../tabs/ConfigPluginOptionsTabTest.java | 58 +++++++
16 files changed, 481 insertions(+), 311 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 becac14f5e..16a6100805 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
@@ -164,7 +164,7 @@ 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.
+Hop Gui can do the same for commits made from within Hop Gui. See
*Configuration -> Plugins -> Linter -> Block Git Commits on Lint Failures*,
which is *off by default*: installing and enabling the linter does not on its
own change how committing behaves.
As with git's own `pre-commit` hook, only commits you author are checked.
Reverting a file and cherry-picking a file are not, because both replay
content that is already in the history — and reverting is often how you fix a
lint failure in the first place.
diff --git a/docs/hop-user-manual/modules/ROOT/pages/linting/index.adoc
b/docs/hop-user-manual/modules/ROOT/pages/linting/index.adoc
index 49814a6f00..f6259fc9fd 100644
--- a/docs/hop-user-manual/modules/ROOT/pages/linting/index.adoc
+++ b/docs/hop-user-manual/modules/ROOT/pages/linting/index.adoc
@@ -105,7 +105,7 @@ A rule's severity is part of the rule, and a project can
override it.
== Configuration
-Linter behaviour is configured under *Configuration -> Linter* in Hop Gui, and
stored in `hop-config.json`.
+Linter behaviour is configured under *Configuration -> Plugins -> Linter* in
Hop Gui, and stored in `hop-config.json`.
The linter can be switched off entirely there, and each of its parts — linting
while you edit, the problems bar, the canvas overlays, the checks added to
*Verify*, and blocking git commits — can be switched off individually.
Blocking git commits is *off by default*. Nothing about committing changes
until you switch it on, and see xref:hop-tools/hop-lint.adoc[hop lint] for
which git operations it applies to.
diff --git a/docs/hop-user-manual/modules/ROOT/pages/linting/lint-rules.adoc
b/docs/hop-user-manual/modules/ROOT/pages/linting/lint-rules.adoc
index 94ce4de96d..3c7412f9b9 100644
--- a/docs/hop-user-manual/modules/ROOT/pages/linting/lint-rules.adoc
+++ b/docs/hop-user-manual/modules/ROOT/pages/linting/lint-rules.adoc
@@ -474,7 +474,7 @@ suppress:
`rule: "*"` accepts whatever is reported on that element, including a rule
that starts reporting on it later, which is what a transform filled in at
runtime needs.
It is only accepted together with a `path` or a `source`: on its own it would
be the linter switched off under another name, and it is refused the same way
an entry without a rule is.
-An element whose findings are accepted is drawn with a muted outline on the
canvas rather than nothing at all, so the absence of a warning reads as
somebody's decision rather than as a check that never ran.
+An element whose findings are accepted is drawn with a dashed outline and a
small greyed-out info badge on the canvas rather than nothing at all, so the
absence of a warning reads as somebody's decision rather than as a check that
never ran.
Turn that off with *Mark Ignored Transforms and Actions* in the linter options.
== How the effective rule set is built
diff --git a/docs/hop-user-manual/modules/ROOT/partials/config/all-plugins.adoc
b/docs/hop-user-manual/modules/ROOT/partials/config/all-plugins.adoc
index 72eb0ccfcd..69ef876a8b 100644
--- a/docs/hop-user-manual/modules/ROOT/partials/config/all-plugins.adoc
+++ b/docs/hop-user-manual/modules/ROOT/partials/config/all-plugins.adoc
@@ -335,14 +335,14 @@ These options are stored under `pass:c[googleDrive]` in
`hop-config.json`.
|===
|Option|Description|Default
-|pass:c[Configuration File]
-|pass:c[Path to hop-lint.yml configuration file (leave empty for
project-specific). Accepts variables.]
-|
-
|pass:c[Enable Linter]
|pass:c[Enable or disable the Hop Linter globally]
|`pass:c[true]`
+|pass:c[Lint Automatically While Editing]
+|pass:c[Re-run lint checks shortly after you change an open pipeline or
workflow. When disabled, files are only linted on open, save, or on demand.]
+|`pass:c[true]`
+
|pass:c[Include Lint Rules in Pipeline Verify]
|pass:c[Add YAML/custom lint findings to the native pipeline verify Problems
tab]
|`pass:c[true]`
@@ -355,30 +355,30 @@ These options are stored under `pass:c[googleDrive]` in
`hop-config.json`.
|pass:c[Also run Hops built-in pipeline/workflow verify checks during lint
project/file scans]
|`pass:c[true]`
-|pass:c[Lint Automatically While Editing]
-|pass:c[Re-run lint checks shortly after you change an open pipeline or
workflow. When disabled, files are only linted on open, save, or on demand.]
+|pass:c[Show Lint Indicators on Canvas]
+|pass:c[Show the lint totals overlay (top-left of the canvas) and the severity
badges on transforms/actions in open pipeline and workflow editors]
|`pass:c[true]`
-|pass:c[Block Commits on Warnings]
-|pass:c[When enabled, warnings also block commits. When disabled, only errors
block commits]
-|`pass:c[false]`
+|pass:c[Mark Ignored Transforms and Actions]
+|pass:c[Draw a dashed outline and a small badge on a transform or action whose
lint findings have been marked as expected, so the absence of a warning reads
as a decision rather than as a check that never ran]
+|`pass:c[true]`
+
+|pass:c[Configuration File]
+|pass:c[Path to hop-lint.yml configuration file (leave empty for
project-specific). Accepts variables.]
+|
|pass:c[Block Git Commits on Lint Failures]
|pass:c[When enabled, lint checks run before Explorer git commits and block
the commit if issues are found]
|`pass:c[false]`
+|pass:c[Block Commits on Warnings]
+|pass:c[When enabled, warnings also block commits. When disabled, only errors
block commits]
+|`pass:c[false]`
+
|pass:c[Include Metadata in Pre-Commit Checks]
|pass:c[Also lint staged metadata files (e.g. metadata/**/*.json) during
pre-commit checks]
|`pass:c[true]`
-|pass:c[Mark Ignored Transforms and Actions]
-|pass:c[Draw a muted outline on a transform or action whose lint findings have
been marked as expected, so the absence of a warning reads as a decision rather
than as a check that never ran]
-|`pass:c[true]`
-
-|pass:c[Show Lint Indicators on Canvas]
-|pass:c[Show the lint totals overlay (top-left of the canvas) and the severity
badges on transforms/actions in open pipeline and workflow editors]
-|`pass:c[true]`
-
|===
== Microsoft Azure
diff --git
a/docs/hop-user-manual/modules/ROOT/partials/config/linter-config.adoc
b/docs/hop-user-manual/modules/ROOT/partials/config/linter-config.adoc
index 52f6ad8d69..1a6bd1ac05 100644
--- a/docs/hop-user-manual/modules/ROOT/partials/config/linter-config.adoc
+++ b/docs/hop-user-manual/modules/ROOT/partials/config/linter-config.adoc
@@ -21,14 +21,14 @@ under the License.
|===
|Option|Description|Default
-|pass:c[Configuration File]
-|pass:c[Path to hop-lint.yml configuration file (leave empty for
project-specific). Accepts variables.]
-|
-
|pass:c[Enable Linter]
|pass:c[Enable or disable the Hop Linter globally]
|`pass:c[true]`
+|pass:c[Lint Automatically While Editing]
+|pass:c[Re-run lint checks shortly after you change an open pipeline or
workflow. When disabled, files are only linted on open, save, or on demand.]
+|`pass:c[true]`
+
|pass:c[Include Lint Rules in Pipeline Verify]
|pass:c[Add YAML/custom lint findings to the native pipeline verify Problems
tab]
|`pass:c[true]`
@@ -41,28 +41,28 @@ under the License.
|pass:c[Also run Hops built-in pipeline/workflow verify checks during lint
project/file scans]
|`pass:c[true]`
-|pass:c[Lint Automatically While Editing]
-|pass:c[Re-run lint checks shortly after you change an open pipeline or
workflow. When disabled, files are only linted on open, save, or on demand.]
+|pass:c[Show Lint Indicators on Canvas]
+|pass:c[Show the lint totals overlay (top-left of the canvas) and the severity
badges on transforms/actions in open pipeline and workflow editors]
|`pass:c[true]`
-|pass:c[Block Commits on Warnings]
-|pass:c[When enabled, warnings also block commits. When disabled, only errors
block commits]
-|`pass:c[false]`
+|pass:c[Mark Ignored Transforms and Actions]
+|pass:c[Draw a dashed outline and a small badge on a transform or action whose
lint findings have been marked as expected, so the absence of a warning reads
as a decision rather than as a check that never ran]
+|`pass:c[true]`
+
+|pass:c[Configuration File]
+|pass:c[Path to hop-lint.yml configuration file (leave empty for
project-specific). Accepts variables.]
+|
|pass:c[Block Git Commits on Lint Failures]
|pass:c[When enabled, lint checks run before Explorer git commits and block
the commit if issues are found]
|`pass:c[false]`
+|pass:c[Block Commits on Warnings]
+|pass:c[When enabled, warnings also block commits. When disabled, only errors
block commits]
+|`pass:c[false]`
+
|pass:c[Include Metadata in Pre-Commit Checks]
|pass:c[Also lint staged metadata files (e.g. metadata/**/*.json) during
pre-commit checks]
|`pass:c[true]`
-|pass:c[Mark Ignored Transforms and Actions]
-|pass:c[Draw a muted outline on a transform or action whose lint findings have
been marked as expected, so the absence of a warning reads as a decision rather
than as a check that never ran]
-|`pass:c[true]`
-
-|pass:c[Show Lint Indicators on Canvas]
-|pass:c[Show the lint totals overlay (top-left of the canvas) and the severity
badges on transforms/actions in open pipeline and workflow editors]
-|`pass:c[true]`
-
|===
diff --git
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintCanvasOverlayHelper.java
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintCanvasOverlayHelper.java
index 129957ea52..d00da4c283 100644
---
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintCanvasOverlayHelper.java
+++
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintCanvasOverlayHelper.java
@@ -251,18 +251,27 @@ public final class LintCanvasOverlayHelper {
}
/**
- * A muted outline on an element whose findings have been accepted.
+ * Mark a transform or action whose findings were ignored on purpose.
*
- * <p>Deliberately quiet: no badge, no colour that competes with a real
finding. It exists so that
- * the absence of a warning reads as a decision rather than as a check that
never ran.
+ * <p>A dashed outline set off from the icon, with a greyed-out info badge
in the corner where an
+ * error badge would sit, in grey so it does not compete with a real
finding. The solid 1px grey
+ * outline it replaces was all but the border every transform has, so an
ignored transform looked
+ * like any other.
*/
public static void drawIgnoredOverlay(IGc gc, int x, int y, int iconSize,
boolean selected) {
if (gc == null) {
return;
}
gc.setLineWidth(selected ? 2 : 1);
- gc.setForeground(IGc.EColor.GRAY);
- gc.drawRoundRectangle(x - 2, y - 2, iconSize + 3, iconSize + 3, 8, 8);
+ gc.setForeground(IGc.EColor.DARKGRAY);
+ gc.setLineStyle(IGc.ELineStyle.DASH);
+ gc.drawRoundRectangle(x - 5, y - 5, iconSize + 9, iconSize + 9, 10, 10);
+ gc.setLineStyle(IGc.ELineStyle.SOLID);
+ try {
+ gc.drawImage(IGc.EImage.INFO_DISABLED, x - 8, y + iconSize - 8,
gc.getMagnification());
+ } catch (Exception ignored) {
+ // Icon drawing is best-effort on the canvas overlay
+ }
}
public static void drawOverlay(
diff --git
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintResultGrouping.java
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintResultGrouping.java
index d618ef72eb..32f8dd6d56 100644
---
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintResultGrouping.java
+++
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintResultGrouping.java
@@ -16,7 +16,10 @@
*/
package org.apache.hop.lint;
+import java.io.File;
import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
@@ -101,6 +104,61 @@ public final class LintResultGrouping {
return grouped;
}
+ /** What the results view sorts the findings of a severity on. */
+ public enum SortKey {
+ FILE,
+ RULE,
+ MESSAGE
+ }
+
+ /**
+ * The order of the findings within a severity: by the chosen column, then
by the others, so the
+ * same results always come out in the same order. Findings used to appear
in the order they were
+ * found, which moved a file's findings to the bottom each time it was
linted again.
+ */
+ public static Comparator<LintResult> order(SortKey key, boolean ascending) {
+ // Resolving a path goes through VFS, so each path is resolved once per
comparator and not on
+ // every comparison. It also gives the same path the same key throughout a
sort.
+ Map<String, String> fileNames = new HashMap<>();
+ Comparator<LintResult> byFile =
+ Comparator.comparing(
+ (LintResult r) ->
+ fileNames.computeIfAbsent(text(r.getFileName()), path ->
fileName(path)),
+ String.CASE_INSENSITIVE_ORDER)
+ .thenComparing(r -> text(r.getFileName()),
String.CASE_INSENSITIVE_ORDER);
+ Comparator<LintResult> byRule =
+ Comparator.comparing((LintResult r) -> text(r.getRuleId()),
String.CASE_INSENSITIVE_ORDER);
+ Comparator<LintResult> byMessage =
+ Comparator.comparing((LintResult r) -> text(r.getMessage()),
String.CASE_INSENSITIVE_ORDER);
+
+ Comparator<LintResult> order =
+ switch (key) {
+ case RULE -> byRule.thenComparing(byFile).thenComparing(byMessage);
+ case MESSAGE ->
byMessage.thenComparing(byFile).thenComparing(byRule);
+ default -> byFile.thenComparing(byRule).thenComparing(byMessage);
+ };
+ return ascending ? order : order.reversed();
+ }
+
+ /** ERROR, WARNING and INFO in that order, whatever came first; anything
else after them. */
+ public static Comparator<String> severityOrder() {
+ List<String> known = List.of("ERROR", "WARNING", "INFO");
+ return Comparator.comparing(
+ (String severity) -> {
+ int index = known.indexOf(severity == null ? "" :
severity.toUpperCase());
+ return index < 0 ? known.size() : index;
+ })
+ .thenComparing(severity -> text(severity),
String.CASE_INSENSITIVE_ORDER);
+ }
+
+ private static String fileName(String path) {
+ return path.isEmpty() ? path : new
File(LintPathUtils.normalizePath(path)).getName();
+ }
+
+ private static String text(String value) {
+ return value == null ? "" : value;
+ }
+
public static int countBySeverity(List<LintResult> results, String severity)
{
return (int) results.stream().filter(result ->
severity.equals(result.getSeverity())).count();
}
diff --git
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintResultsPanel.java
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintResultsPanel.java
index c5c4fec09c..6b6bc49d7e 100644
--- a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintResultsPanel.java
+++ b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintResultsPanel.java
@@ -47,6 +47,8 @@ public class LintResultsPanel extends Composite implements
LintResultsManager.Li
private LintResultsManager resultsManager;
private Tree resultsTree;
+ private LintResultGrouping.SortKey sortKey = LintResultGrouping.SortKey.FILE;
+ private boolean sortAscending = true;
private Text detailsText;
private Label statusLabel;
private String fileFilter;
@@ -140,6 +142,13 @@ public class LintResultsPanel extends Composite implements
LintResultsManager.Li
fileColumn.setText(BaseMessages.getString(PKG,
"LintResultsPanel.Column.File"));
fileColumn.setWidth(200);
+ ruleColumn.addListener(SWT.Selection, e -> sortOn(ruleColumn,
LintResultGrouping.SortKey.RULE));
+ messageColumn.addListener(
+ SWT.Selection, e -> sortOn(messageColumn,
LintResultGrouping.SortKey.MESSAGE));
+ fileColumn.addListener(SWT.Selection, e -> sortOn(fileColumn,
LintResultGrouping.SortKey.FILE));
+ resultsTree.setSortColumn(fileColumn);
+ resultsTree.setSortDirection(SWT.UP);
+
// Details panel
Group detailsGroup = new Group(sashForm, SWT.NONE);
detailsGroup.setText(BaseMessages.getString(PKG,
"LintResultsPanel.Group.Details"));
@@ -219,81 +228,50 @@ public class LintResultsPanel extends Composite
implements LintResultsManager.Li
displayedResults.size(), scope, errorCount,
warningCount));
Map<String, List<LintResult>> resultsBySeverity =
groupBySeverity(displayedResults);
-
- // Add error group
- if (resultsBySeverity.containsKey("ERROR")) {
- TreeItem errorGroup = new TreeItem(resultsTree, SWT.NONE);
- errorGroup.setText(
- new String[] {
- "ERROR", "", "Errors (" +
resultsBySeverity.get("ERROR").size() + ")", ""
- });
- errorGroup.setExpanded(true);
-
- for (LintResult result : resultsBySeverity.get("ERROR")) {
- TreeItem item = new TreeItem(errorGroup, SWT.NONE);
- item.setText(
- new String[] {
- result.getSeverity(),
- result.getRuleId(),
- result.getMessage(),
- LintEditorGraphHelper.displayName(result.getFileName())
- });
- item.setData(result);
- }
+ List<String> severities = new
java.util.ArrayList<>(resultsBySeverity.keySet());
+ severities.sort(LintResultGrouping.severityOrder());
+ for (String severity : severities) {
+ addSeverityGroup(severity, resultsBySeverity.get(severity));
}
+ });
+ }
- // Add warning group
- if (resultsBySeverity.containsKey("WARNING")) {
- TreeItem warningGroup = new TreeItem(resultsTree, SWT.NONE);
- warningGroup.setText(
- new String[] {
- "WARNING",
- "",
- "Warnings (" + resultsBySeverity.get("WARNING").size() +
")",
- ""
- });
- warningGroup.setExpanded(true);
-
- for (LintResult result : resultsBySeverity.get("WARNING")) {
- TreeItem item = new TreeItem(warningGroup, SWT.NONE);
- item.setText(
- new String[] {
- result.getSeverity(),
- result.getRuleId(),
- result.getMessage(),
- LintEditorGraphHelper.displayName(result.getFileName())
- });
- item.setData(result);
- }
- }
+ private void addSeverityGroup(String severity, List<LintResult> results) {
+ String label =
+ switch (severity) {
+ case "ERROR" -> "Errors";
+ case "WARNING" -> "Warnings";
+ default -> severity;
+ };
+ TreeItem group = new TreeItem(resultsTree, SWT.NONE);
+ group.setText(new String[] {severity, "", label + " (" + results.size() +
")", ""});
+
+ List<LintResult> sorted = new java.util.ArrayList<>(results);
+ sorted.sort(LintResultGrouping.order(sortKey, sortAscending));
+ for (LintResult result : sorted) {
+ TreeItem item = new TreeItem(group, SWT.NONE);
+ item.setText(
+ new String[] {
+ result.getSeverity(),
+ result.getRuleId(),
+ result.getMessage(),
+ LintEditorGraphHelper.displayName(result.getFileName())
+ });
+ item.setData(result);
+ }
+ group.setExpanded(true);
+ }
- // Add other severity groups
- for (Map.Entry<String, List<LintResult>> entry :
resultsBySeverity.entrySet()) {
- if (!"ERROR".equals(entry.getKey()) &&
!"WARNING".equals(entry.getKey())) {
- TreeItem group = new TreeItem(resultsTree, SWT.NONE);
- group.setText(
- new String[] {
- entry.getKey(),
- "",
- entry.getKey() + " (" + entry.getValue().size() + ")",
- ""
- });
- group.setExpanded(true);
-
- for (LintResult result : entry.getValue()) {
- TreeItem item = new TreeItem(group, SWT.NONE);
- item.setText(
- new String[] {
- result.getSeverity(),
- result.getRuleId(),
- result.getMessage(),
- result.getFileName()
- });
- item.setData(result);
- }
- }
- }
- });
+ /**
+ * Sort on a column, or turn the order round when it is already the one
sorted on. The severity
+ * groups keep their order: errors first.
+ */
+ private void sortOn(TreeColumn column, LintResultGrouping.SortKey key) {
+ sortAscending = key != sortKey || !sortAscending;
+ sortKey = key;
+ resultsTree.setSortColumn(column);
+ resultsTree.setSortDirection(sortAscending ? SWT.UP : SWT.DOWN);
+ refreshResults();
}
private List<LintResult> getDisplayedResults() {
diff --git
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintSuppressDialog.java
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintSuppressDialog.java
index 1a46f8fca2..0d4fc3757c 100644
---
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintSuppressDialog.java
+++
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintSuppressDialog.java
@@ -84,10 +84,10 @@ public class LintSuppressDialog extends Dialog {
createContents();
- shell.setSize(560, 420);
+ shell.setSize(640, 440);
shell.setLocation(
- parent.getLocation().x + (parent.getSize().x - 560) / 2,
- parent.getLocation().y + (parent.getSize().y - 420) / 2);
+ parent.getLocation().x + (parent.getSize().x - 640) / 2,
+ parent.getLocation().y + (parent.getSize().y - 440) / 2);
shell.open();
Display display = parent.getDisplay();
@@ -102,7 +102,8 @@ public class LintSuppressDialog extends Dialog {
private void createContents() {
int margin = 10;
- Label header = new Label(shell, SWT.LEFT);
+ // Wrapped: at the default width the line was cut off before it said where
the decision goes.
+ Label header = new Label(shell, SWT.LEFT | SWT.WRAP);
header.setText(BaseMessages.getString(PKG,
"LintSuppressDialog.Label.Element", elementName));
FormData headerData = new FormData();
headerData.left = new FormAttachment(0, margin);
@@ -138,10 +139,10 @@ public class LintSuppressDialog extends Dialog {
allRulesButton.setLayoutData(allRulesData);
listedRulesButton = new Button(shell, SWT.RADIO);
- listedRulesButton.setText(
- BaseMessages.getString(PKG, "LintSuppressDialog.Scope.ListedRules",
ruleList()));
+ // The rules are listed in the findings above; in the label a long list
was cut off.
+ listedRulesButton.setText(BaseMessages.getString(PKG,
"LintSuppressDialog.Scope.ListedRules"));
listedRulesButton.setToolTipText(
- BaseMessages.getString(PKG,
"LintSuppressDialog.Scope.ListedRules.ToolTip"));
+ BaseMessages.getString(PKG,
"LintSuppressDialog.Scope.ListedRules.ToolTip", ruleList()));
listedRulesButton.setEnabled(!ruleIdsOfFindings().isEmpty());
FormData listedRulesData = new FormData();
listedRulesData.left = new FormAttachment(0, margin);
@@ -149,9 +150,12 @@ public class LintSuppressDialog extends Dialog {
listedRulesData.top = new FormAttachment(allRulesButton, margin / 2);
listedRulesButton.setLayoutData(listedRulesData);
- // Accepting everything on the element is the metadata injection case,
which is the reason
- // this dialog exists; naming the rules is the careful option for anyone
who wants it.
- allRulesButton.setSelection(true);
+ // Naming the rules reported now is the default: ignoring everything on
the element also
+ // ignores rules added later, security ones included. That broad option is
for an element
+ // filled in at runtime, such as by metadata injection, and it is a
deliberate choice.
+ boolean rulesToName = !ruleIdsOfFindings().isEmpty();
+ listedRulesButton.setSelection(rulesToName);
+ allRulesButton.setSelection(!rulesToName);
Label reasonLabel = new Label(shell, SWT.LEFT);
reasonLabel.setText(BaseMessages.getString(PKG,
"LintSuppressDialog.Label.Reason"));
diff --git
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LinterConfigPlugin.java
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LinterConfigPlugin.java
index 01b99b4928..b72ac48c6c 100644
---
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LinterConfigPlugin.java
+++
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LinterConfigPlugin.java
@@ -116,15 +116,29 @@ public class LinterConfigPlugin implements
IConfigOptions, IGuiPluginCompositeWi
@Override
public void widgetsPopulated(GuiCompositeWidgets compositeWidgets) {
- // Nothing to do.
+ enableDependentWidgets(compositeWidgets);
}
@Override
public void widgetModified(
GuiCompositeWidgets compositeWidgets, Control changedWidget, String
widgetId) {
+ enableDependentWidgets(compositeWidgets);
persistContents(compositeWidgets);
}
+ /**
+ * Blocking commits on warnings only means anything when commits are blocked
at all, so it is
+ * greyed out while they are not. "Include Metadata in Pre-Commit Checks"
stays available: the
+ * background lint reads it as well.
+ */
+ static void enableDependentWidgets(GuiCompositeWidgets compositeWidgets) {
+ Map<String, Control> widgets = compositeWidgets.getWidgetsMap();
+ if (widgets.get("linter-pre-commit-enabled") instanceof Button preCommit
+ && widgets.get("linter-pre-commit-block-warnings") instanceof Button
blockWarnings) {
+ blockWarnings.setEnabled(preCommit.getSelection());
+ }
+ }
+
/**
* Write what the user chose back into hop-config.json.
*
@@ -256,6 +270,7 @@ public class LinterConfigPlugin implements IConfigOptions,
IGuiPluginCompositeWi
// Global linter settings
@GuiWidgetElement(
id = "linter-enabled",
+ order = "0100",
parentId = ConfigPluginOptionsTab.GUI_WIDGETS_PARENT_ID,
type = GuiElementType.CHECKBOX,
label = "i18n::LinterConfigPlugin.Option.Enabled.Label",
@@ -268,6 +283,7 @@ public class LinterConfigPlugin implements IConfigOptions,
IGuiPluginCompositeWi
@GuiWidgetElement(
id = "linter-lint-on-edit",
+ order = "0110",
parentId = ConfigPluginOptionsTab.GUI_WIDGETS_PARENT_ID,
type = GuiElementType.CHECKBOX,
label = "i18n::LinterConfigPlugin.Option.LintOnEdit.Label",
@@ -280,6 +296,7 @@ public class LinterConfigPlugin implements IConfigOptions,
IGuiPluginCompositeWi
@GuiWidgetElement(
id = "linter-show-problems-bar",
+ order = "0300",
parentId = ConfigPluginOptionsTab.GUI_WIDGETS_PARENT_ID,
type = GuiElementType.CHECKBOX,
label = "i18n::LinterConfigPlugin.Option.ShowIndicators.Label",
@@ -292,6 +309,7 @@ public class LinterConfigPlugin implements IConfigOptions,
IGuiPluginCompositeWi
@GuiWidgetElement(
id = "linter-show-ignored-markers",
+ order = "0310",
parentId = ConfigPluginOptionsTab.GUI_WIDGETS_PARENT_ID,
type = GuiElementType.CHECKBOX,
label = "i18n::LinterConfigPlugin.Option.ShowIgnoredMarkers.Label",
@@ -304,6 +322,7 @@ public class LinterConfigPlugin implements IConfigOptions,
IGuiPluginCompositeWi
@GuiWidgetElement(
id = "linter-config-file",
+ order = "0400",
parentId = ConfigPluginOptionsTab.GUI_WIDGETS_PARENT_ID,
type = GuiElementType.FILENAME,
variables = true,
@@ -316,6 +335,7 @@ public class LinterConfigPlugin implements IConfigOptions,
IGuiPluginCompositeWi
@GuiWidgetElement(
id = "linter-pre-commit-enabled",
+ order = "0500",
parentId = ConfigPluginOptionsTab.GUI_WIDGETS_PARENT_ID,
type = GuiElementType.CHECKBOX,
label = "i18n::LinterConfigPlugin.Option.PreCommit.Label",
@@ -328,6 +348,7 @@ public class LinterConfigPlugin implements IConfigOptions,
IGuiPluginCompositeWi
@GuiWidgetElement(
id = "linter-pre-commit-block-warnings",
+ order = "0510",
parentId = ConfigPluginOptionsTab.GUI_WIDGETS_PARENT_ID,
type = GuiElementType.CHECKBOX,
label = "i18n::LinterConfigPlugin.Option.PreCommitWarnings.Label",
@@ -340,6 +361,7 @@ public class LinterConfigPlugin implements IConfigOptions,
IGuiPluginCompositeWi
@GuiWidgetElement(
id = "linter-pre-commit-include-metadata",
+ order = "0520",
parentId = ConfigPluginOptionsTab.GUI_WIDGETS_PARENT_ID,
type = GuiElementType.CHECKBOX,
label = "i18n::LinterConfigPlugin.Option.PreCommitMetadata.Label",
@@ -352,6 +374,7 @@ public class LinterConfigPlugin implements IConfigOptions,
IGuiPluginCompositeWi
@GuiWidgetElement(
id = "linter-include-in-pipeline-verify",
+ order = "0200",
parentId = ConfigPluginOptionsTab.GUI_WIDGETS_PARENT_ID,
type = GuiElementType.CHECKBOX,
label = "i18n::LinterConfigPlugin.Option.PipelineVerify.Label",
@@ -364,6 +387,7 @@ public class LinterConfigPlugin implements IConfigOptions,
IGuiPluginCompositeWi
@GuiWidgetElement(
id = "linter-include-in-workflow-verify",
+ order = "0210",
parentId = ConfigPluginOptionsTab.GUI_WIDGETS_PARENT_ID,
type = GuiElementType.CHECKBOX,
label = "i18n::LinterConfigPlugin.Option.WorkflowVerify.Label",
@@ -376,6 +400,7 @@ public class LinterConfigPlugin implements IConfigOptions,
IGuiPluginCompositeWi
@GuiWidgetElement(
id = "linter-include-native-checks",
+ order = "0220",
parentId = ConfigPluginOptionsTab.GUI_WIDGETS_PARENT_ID,
type = GuiElementType.CHECKBOX,
label = "i18n::LinterConfigPlugin.Option.NativeChecks.Label",
diff --git
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LinterGuiPlugin.java
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LinterGuiPlugin.java
index 487aa78b91..c0d7f7312c 100644
--- a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LinterGuiPlugin.java
+++ b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LinterGuiPlugin.java
@@ -99,7 +99,9 @@ public class LinterGuiPlugin {
}
String projectPath = variables.getVariable("PROJECT_HOME");
- String projectName = variables.getVariable("PROJECT_NAME");
+ // HOP_PROJECT_NAME is what the projects plugin sets; nothing sets
PROJECT_NAME, so every
+ // run was reported as "Unknown Project".
+ String projectName = variables.getVariable("HOP_PROJECT_NAME");
if (Utils.isEmpty(projectPath)) {
// No project loaded, show error message
@@ -122,7 +124,8 @@ public class LinterGuiPlugin {
final String finalProjectPath = projectPath;
final IVariables finalVariables = variables;
final HopGui finalHopGui = hopGui;
- final String finalProjectName = projectName != null ? projectName :
"Unknown Project";
+ final String finalProjectName =
+ !Utils.isEmpty(projectName) ? projectName : new
java.io.File(projectPath).getName();
// Run linter in background thread
BackgroundThreadFacade.start(
@@ -143,9 +146,6 @@ public class LinterGuiPlugin {
return;
}
- // Generate rule summary
- Map<String, HopLinter.RuleSummary> ruleSummary =
linter.generateRuleSummary(results);
-
// Process and display results on UI thread
Display.getDefault()
.asyncExec(
@@ -156,8 +156,7 @@ public class LinterGuiPlugin {
LintProblemsBarManager.getInstance().refreshAllOpenEditors();
LinterGuiPlugin plugin = new LinterGuiPlugin();
- plugin.displayResults(
- finalHopGui, results, ruleSummary,
totalLintTime, finalProjectName);
+ plugin.displayResults(results, totalLintTime,
finalProjectName);
} catch (Exception e) {
LogChannel.GENERAL.logError(
"Error displaying results: " + e.getMessage(),
e);
@@ -208,33 +207,20 @@ public class LinterGuiPlugin {
}
/**
- * Display the linting results to the user
+ * Report a project lint in the Hop log. The findings themselves are in Show
Lint Results and on
+ * the canvas.
*
- * @param hopGui The HopGui instance
- * @param results List of lint results
- * @param ruleSummary Summary of results by rule
- * @param executionTimeMs Total execution time in milliseconds
- * @param projectName Name of the project that was linted
+ * <p>Each run also wrote a hop-lint-results_<timestamp>.txt into the
project root. They
+ * piled up, were picked up by a plain "git add .", and carried whatever the
findings said.
*/
- private void displayResults(
- HopGui hopGui,
- List<LintResult> results,
- Map<String, HopLinter.RuleSummary> ruleSummary,
- long executionTimeMs,
- String projectName) {
- // Write detailed results to file instead of console
- String projectPath = writeResultsToFile(results, ruleSummary,
executionTimeMs, projectName);
-
- // Log summary to the Hop log
+ private void displayResults(List<LintResult> results, long executionTimeMs,
String projectName) {
LogChannel.GENERAL.logBasic("=== LINT RESULTS FOR PROJECT: " + projectName
+ " ===");
LogChannel.GENERAL.logBasic("Execution time: " +
formatExecutionTime(executionTimeMs));
- LogChannel.GENERAL.logBasic("Detailed results written to: " + projectPath);
if (results.isEmpty()) {
LogChannel.GENERAL.logBasic(
"✓ No issues found! Your project follows all configured best
practices.");
} else {
- // Group results by severity
Map<String, List<LintResult>> resultsBySeverity =
results.stream().collect(Collectors.groupingBy(LintResult::getSeverity));
@@ -248,157 +234,7 @@ public class LinterGuiPlugin {
}
LogChannel.GENERAL.logBasic("=== END LINT RESULTS ===");
-
- if (results.isEmpty()) {
- LintResultsUi.logSummary(results, projectName);
- } else {
- LintResultsUi.logSummary(results, projectName);
- }
- }
-
- /**
- * Write detailed lint results to a file in the project root
- *
- * @param results All lint results
- * @param ruleSummary Summary by rule
- * @param executionTimeMs Execution time
- * @param projectName Project name
- * @return Path to the results file
- */
- private String writeResultsToFile(
- List<LintResult> results,
- Map<String, HopLinter.RuleSummary> ruleSummary,
- long executionTimeMs,
- String projectName) {
- try {
- // Get project path from HopGui variables
- HopGui hopGui = HopGui.peekInstance();
- IVariables variables = hopGui.getVariables();
- String projectPath = variables.getVariable("PROJECT_HOME");
-
- if (Utils.isEmpty(projectPath)) {
- // Fallback to current working directory
- projectPath = System.getProperty("user.dir");
- }
-
- // Create results file with timestamp
- String timestamp =
- java.time.LocalDateTime.now()
-
.format(java.time.format.DateTimeFormatter.ofPattern("yyyy-MM-dd_HH-mm-ss"));
- String fileName = "hop-lint-results_" + timestamp + ".txt";
- java.io.File resultsFile = new java.io.File(projectPath, fileName);
-
- // Write results to file
- try (java.io.PrintWriter writer =
- new java.io.PrintWriter(
- new java.io.FileWriter(resultsFile,
java.nio.charset.StandardCharsets.UTF_8))) {
-
- writer.println("=== HOP LINT CHECKER RESULTS ===");
- writer.println("Project: " + projectName);
- writer.println("Execution time: " +
formatExecutionTime(executionTimeMs));
- writer.println(
- "Generated: "
- + java.time.LocalDateTime.now()
-
.format(java.time.format.DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss")));
- writer.println();
-
- // Write rule summary
- writer.println("RULE SUMMARY:");
- ruleSummary.entrySet().stream()
- .sorted(Map.Entry.comparingByKey())
- .forEach(entry -> writer.println(" " +
entry.getValue().toString()));
- writer.println();
-
- if (results.isEmpty()) {
- writer.println("✓ No issues found! Your project follows all
configured best practices.");
- } else {
- // Group results by severity
- Map<String, List<LintResult>> resultsBySeverity =
-
results.stream().collect(Collectors.groupingBy(LintResult::getSeverity));
-
- int errorCount = resultsBySeverity.getOrDefault("ERROR",
List.of()).size();
- int warningCount = resultsBySeverity.getOrDefault("WARNING",
List.of()).size();
-
- writer.println("SUMMARY:");
- writer.println(" Total issues: " + results.size());
- writer.println(" Errors: " + errorCount);
- writer.println(" Warnings: " + warningCount);
- writer.println();
-
- // Write detailed results
- writer.println("DETAILED RESULTS:");
- writer.println();
-
- // Write errors first
- List<LintResult> errors = resultsBySeverity.get("ERROR");
- if (errors != null && !errors.isEmpty()) {
- writer.println("ERRORS:");
- for (LintResult error : errors) {
- writer.println(
- " [ERROR] "
- + error.getRuleId()
- + " - "
- + error.getRuleName()
- + ": "
- + error.getMessage()
- + " (File: "
- + error.getFileName()
- + ")");
- }
- writer.println();
- }
-
- // Write warnings
- List<LintResult> warnings = resultsBySeverity.get("WARNING");
- if (warnings != null && !warnings.isEmpty()) {
- writer.println("WARNINGS:");
- for (LintResult warning : warnings) {
- writer.println(
- " [WARNING] "
- + warning.getRuleId()
- + " - "
- + warning.getRuleName()
- + ": "
- + warning.getMessage()
- + " (File: "
- + warning.getFileName()
- + ")");
- }
- writer.println();
- }
-
- // Write any other severity levels
- for (Map.Entry<String, List<LintResult>> entry :
resultsBySeverity.entrySet()) {
- if (!"ERROR".equals(entry.getKey()) &&
!"WARNING".equals(entry.getKey())) {
- writer.println(entry.getKey() + ":");
- for (LintResult result : entry.getValue()) {
- writer.println(
- " ["
- + result.getSeverity()
- + "] "
- + result.getRuleId()
- + " - "
- + result.getRuleName()
- + ": "
- + result.getMessage()
- + " (File: "
- + result.getFileName()
- + ")");
- }
- writer.println();
- }
- }
- }
-
- writer.println("=== END LINT RESULTS ===");
- }
-
- return resultsFile.getAbsolutePath();
-
- } catch (Exception e) {
- LogChannel.GENERAL.logError("Error writing results to file: " +
e.getMessage(), e);
- return "Error writing to file: " + e.getMessage();
- }
+ LintResultsUi.logSummary(results, projectName);
}
/**
diff --git
a/plugins/misc/lint/src/main/resources/org/apache/hop/lint/messages/messages_en_US.properties
b/plugins/misc/lint/src/main/resources/org/apache/hop/lint/messages/messages_en_US.properties
index 94a2ec2775..cc2dc1aa7b 100644
---
a/plugins/misc/lint/src/main/resources/org/apache/hop/lint/messages/messages_en_US.properties
+++
b/plugins/misc/lint/src/main/resources/org/apache/hop/lint/messages/messages_en_US.properties
@@ -39,7 +39,7 @@ LinterConfigPlugin.Option.LintOnEdit.ToolTip=Re-run lint
checks shortly after yo
LinterConfigPlugin.Option.ShowIndicators.Label=Show Lint Indicators on Canvas
LinterConfigPlugin.Option.ShowIndicators.ToolTip=Show the lint totals overlay
(top-left of the canvas) and the severity badges on transforms/actions in open
pipeline and workflow editors
LinterConfigPlugin.Option.ShowIgnoredMarkers.Label=Mark Ignored Transforms and
Actions
-LinterConfigPlugin.Option.ShowIgnoredMarkers.ToolTip=Draw a muted outline on a
transform or action whose lint findings have been marked as expected, so the
absence of a warning reads as a decision rather than as a check that never ran
+LinterConfigPlugin.Option.ShowIgnoredMarkers.ToolTip=Draw a dashed outline and
a small badge on a transform or action whose lint findings have been marked as
expected, so the absence of a warning reads as a decision rather than as a
check that never ran
LinterConfigPlugin.Option.ConfigFile.Label=Configuration File
LinterConfigPlugin.Option.ConfigFile.ToolTip=Path to hop-lint.yml
configuration file (leave empty for project-specific)
LinterConfigPlugin.Option.PreCommit.Label=Block Git Commits on Lint Failures
@@ -147,11 +147,11 @@ LintSuppressDialog.Label.Findings=Currently reported:
LintSuppressDialog.NoFindings=Nothing is currently reported here. Anything
found later will be ignored as well.
LintSuppressDialog.Scope.AllRules=Ignore every finding on this element, now
and later
LintSuppressDialog.Scope.AllRules.ToolTip=Use this for an element that is
filled in at runtime, for example by metadata injection: whatever a check
reports about it at design time is expected.
-LintSuppressDialog.Scope.ListedRules=Ignore only these rules: {0}
-LintSuppressDialog.Scope.ListedRules.ToolTip=Keep checking this element, but
accept the rules reported right now. A new kind of problem here will still be
reported.
+LintSuppressDialog.Scope.ListedRules=Ignore only the rules reported now
+LintSuppressDialog.Scope.ListedRules.ToolTip=Keep checking this element, but
accept {0}. A new kind of problem here will still be reported.
LintSuppressDialog.Label.Reason=Reason (required):
LintSuppressDialog.ReasonRequired.Title=Reason Required
-LintSuppressDialog.ReasonRequired.Message=Please say why these findings are
expected. It is stored with the pipeline or workflow, so whoever reads it next
knows this was a decision rather than an oversight.
+LintSuppressDialog.ReasonRequired.Message=Please say why these findings are
expected. It is recorded in the project''s hop-lint.yml, so whoever reads it
next knows this was a decision rather than an oversight.
# ---------------------------------------------------------------------------
# Keeping a file or folder out of linting
diff --git
a/plugins/misc/lint/src/test/java/org/apache/hop/lint/LintResultsViewTest.java
b/plugins/misc/lint/src/test/java/org/apache/hop/lint/LintResultsViewTest.java
new file mode 100644
index 0000000000..caa9461076
--- /dev/null
+++
b/plugins/misc/lint/src/test/java/org/apache/hop/lint/LintResultsViewTest.java
@@ -0,0 +1,131 @@
+/*
+ * 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.mockito.ArgumentMatchers.anyFloat;
+import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.inOrder;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import org.apache.hop.core.gui.IGc;
+import org.apache.hop.ui.core.gui.GuiCompositeWidgets;
+import org.eclipse.swt.widgets.Button;
+import org.eclipse.swt.widgets.Control;
+import org.junit.jupiter.api.Test;
+import org.mockito.InOrder;
+
+/**
+ * Show Lint Results, the ignored marker and the linter settings.
+ *
+ * @see <a href="https://github.com/apache/hop/issues/8735";>#8735</a>
+ */
+public class LintResultsViewTest {
+
+ private static LintResult finding(String file, String ruleId) {
+ return new LintResult(ruleId, ruleId, "WARNING", "message " + ruleId,
file);
+ }
+
+ private static List<String> sorted(
+ List<LintResult> results, LintResultGrouping.SortKey key, boolean
ascending) {
+ List<LintResult> copy = new ArrayList<>(results);
+ copy.sort(LintResultGrouping.order(key, ascending));
+ return copy.stream()
+ .map(r -> new java.io.File(r.getFileName()).getName() + " " +
r.getRuleId())
+ .toList();
+ }
+
+ private static final List<LintResult> RESULTS =
+ List.of(
+ finding("/project/load/b.hpl", "STRUCT-003"),
+ finding("/project/main.hwf", "DOC-002"),
+ finding("/project/load/a.hpl", "NAMING-004"),
+ finding("/project/load/b.hpl", "DOC-001"));
+
+ /** Files came in no clear order, main.hwf between two .hpl files, with
rules mixed per file. */
+ @Test
+ public void resultsAreOrderedByFileThenRule() {
+ assertEquals(
+ List.of("a.hpl NAMING-004", "b.hpl DOC-001", "b.hpl STRUCT-003",
"main.hwf DOC-002"),
+ sorted(RESULTS, LintResultGrouping.SortKey.FILE, true));
+ }
+
+ @Test
+ public void resultsCanBeOrderedByRuleEitherWay() {
+ List<String> byRule = sorted(RESULTS, LintResultGrouping.SortKey.RULE,
true);
+ assertEquals(
+ List.of("b.hpl DOC-001", "main.hwf DOC-002", "a.hpl NAMING-004",
"b.hpl STRUCT-003"),
+ byRule);
+ assertEquals(
+ byRule.reversed(), sorted(RESULTS, LintResultGrouping.SortKey.RULE,
false), "reversed");
+ }
+
+ @Test
+ public void errorsComeFirst() {
+ List<String> severities = new ArrayList<>(List.of("INFO", "CUSTOM",
"WARNING", "ERROR"));
+ severities.sort(LintResultGrouping.severityOrder());
+ assertEquals(List.of("ERROR", "WARNING", "INFO", "CUSTOM"), severities);
+ }
+
+ /**
+ * A 1px grey rounded outline was all but the normal transform border, so an
ignored transform
+ * looked like any other.
+ */
+ @Test
+ public void anIgnoredTransformGetsADashedOutlineAndABadge() throws Exception
{
+ IGc gc = mock(IGc.class);
+ when(gc.getMagnification()).thenReturn(1f);
+
+ LintCanvasOverlayHelper.drawIgnoredOverlay(gc, 100, 100, 32, false);
+
+ InOrder order = inOrder(gc);
+ order.verify(gc).setLineStyle(IGc.ELineStyle.DASH);
+ order.verify(gc).drawRoundRectangle(anyInt(), anyInt(), anyInt(),
anyInt(), anyInt(), anyInt());
+ order.verify(gc).setLineStyle(IGc.ELineStyle.SOLID);
+ verify(gc).drawImage(eq(IGc.EImage.INFO_DISABLED), anyInt(), anyInt(),
anyFloat());
+ }
+
+ /**
+ * "Block Commits on Warnings" means nothing while commits are not blocked,
and stayed enabled
+ * when they were not.
+ */
+ @Test
+ public void blockingOnWarningsFollowsBlockingCommits() {
+ Button preCommit = mock(Button.class);
+ Button blockWarnings = mock(Button.class);
+ GuiCompositeWidgets widgets = mock(GuiCompositeWidgets.class);
+ when(widgets.getWidgetsMap())
+ .thenReturn(
+ Map.<String, Control>of(
+ "linter-pre-commit-enabled", preCommit,
+ "linter-pre-commit-block-warnings", blockWarnings));
+
+ when(preCommit.getSelection()).thenReturn(false);
+ LinterConfigPlugin.enableDependentWidgets(widgets);
+ verify(blockWarnings).setEnabled(false);
+
+ when(preCommit.getSelection()).thenReturn(true);
+ LinterConfigPlugin.enableDependentWidgets(widgets);
+ verify(blockWarnings).setEnabled(true);
+ }
+}
diff --git
a/plugins/misc/lint/src/test/java/org/apache/hop/lint/LintSuppressDialogDefaultTest.java
b/plugins/misc/lint/src/test/java/org/apache/hop/lint/LintSuppressDialogDefaultTest.java
new file mode 100644
index 0000000000..9879ce27c5
--- /dev/null
+++
b/plugins/misc/lint/src/test/java/org/apache/hop/lint/LintSuppressDialogDefaultTest.java
@@ -0,0 +1,62 @@
+/*
+ * 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.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.List;
+import java.util.concurrent.atomic.AtomicBoolean;
+import org.apache.hop.ui.testing.SwtBotTestBase;
+import org.eclipse.swtbot.swt.finder.SWTBot;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+
+/**
+ * The Ignore Lint Findings dialog.
+ *
+ * <p>It defaulted to ignoring every finding on the element, now and later,
which also silences
+ * rules added afterwards, security ones included.
+ *
+ * @see <a href="https://github.com/apache/hop/issues/8735";>#8735</a>
+ */
+@Tag("uitest")
+class LintSuppressDialogDefaultTest extends SwtBotTestBase {
+
+ @Test
+ void ignoringOnlyTheReportedRulesIsTheDefault() {
+ List<LintResult> findings =
+ List.of(
+ new LintResult(
+ "SEC-002", "Hardcoded Password", "ERROR", "hardcoded value",
"/p/load.hpl"));
+ AtomicBoolean listedSelected = new AtomicBoolean();
+ AtomicBoolean allSelected = new AtomicBoolean();
+
+ withDialog(
+ parent -> new LintSuppressDialog(parent, "HTTP client",
findings).open(),
+ bot -> {
+ SWTBot dialog = bot.shell("Ignore Lint Findings").bot();
+ listedSelected.set(dialog.radio("Ignore only the rules reported
now").isSelected());
+ allSelected.set(
+ dialog.radio("Ignore every finding on this element, now and
later").isSelected());
+ dialog.button(buttonLabel("System.Button.Cancel")).click();
+ });
+
+ assertTrue(listedSelected.get(), "only the reported rules");
+ assertFalse(allSelected.get(), "not everything, now and later");
+ }
+}
diff --git
a/ui/src/main/java/org/apache/hop/ui/hopgui/perspective/configuration/tabs/ConfigPluginOptionsTab.java
b/ui/src/main/java/org/apache/hop/ui/hopgui/perspective/configuration/tabs/ConfigPluginOptionsTab.java
index f6f3018886..7a86244409 100644
---
a/ui/src/main/java/org/apache/hop/ui/hopgui/perspective/configuration/tabs/ConfigPluginOptionsTab.java
+++
b/ui/src/main/java/org/apache/hop/ui/hopgui/perspective/configuration/tabs/ConfigPluginOptionsTab.java
@@ -144,6 +144,20 @@ public class ConfigPluginOptionsTab {
wPluginsTab.setControl(wPluginsTabComp);
}
+ /**
+ * Let a plugin that listens to its widgets save changes, and tell it the
widgets are filled.
+ * Filling the widgets fires no events, so without that call the plugin
cannot set up which
+ * widgets are enabled until the user changes something.
+ */
+ static void attachWidgetsListener(GuiCompositeWidgets compositeWidgets,
Object pluginSourceData) {
+ if (pluginSourceData instanceof IGuiPluginCompositeWidgetsListener
listener) {
+ // This listener saves the changed values immediately, so we don't have
to worry about that.
+ //
+ compositeWidgets.setWidgetsListener(listener);
+ listener.widgetsPopulated(compositeWidgets);
+ }
+ }
+
/**
* Show settings for a specific plugin. This is called from the tree
selection in
* ConfigurationPerspective.
@@ -181,12 +195,7 @@ public class ConfigPluginOptionsTab {
compositeWidgets.createCompositeWidgets(
pluginSourceData, null, wPluginsComp, GUI_WIDGETS_PARENT_ID, null);
compositeWidgets.setWidgetsContents(pluginSourceData, wPluginsComp,
GUI_WIDGETS_PARENT_ID);
- if (pluginSourceData
- instanceof IGuiPluginCompositeWidgetsListener
iGuiPluginCompositeWidgetsListener) {
- // This listener saves the changed values immediately, so we don't have
to worry about that.
- //
- compositeWidgets.setWidgetsListener(iGuiPluginCompositeWidgetsListener);
- }
+ attachWidgetsListener(compositeWidgets, pluginSourceData);
wPluginsComp.layout();
wPluginsComp.pack();
diff --git
a/ui/src/test/java/org/apache/hop/ui/hopgui/perspective/configuration/tabs/ConfigPluginOptionsTabTest.java
b/ui/src/test/java/org/apache/hop/ui/hopgui/perspective/configuration/tabs/ConfigPluginOptionsTabTest.java
new file mode 100644
index 0000000000..eafe2fcba7
--- /dev/null
+++
b/ui/src/test/java/org/apache/hop/ui/hopgui/perspective/configuration/tabs/ConfigPluginOptionsTabTest.java
@@ -0,0 +1,58 @@
+/*
+ * 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.ui.hopgui.perspective.configuration.tabs;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.inOrder;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+
+import org.apache.hop.ui.core.gui.GuiCompositeWidgets;
+import org.apache.hop.ui.core.gui.IGuiPluginCompositeWidgetsListener;
+import org.junit.jupiter.api.Test;
+import org.mockito.InOrder;
+
+class ConfigPluginOptionsTabTest {
+
+ /**
+ * Filling the widgets fires no events, so a plugin only learns they are
filled from
+ * widgetsPopulated. Without it, widgets that depend on others kept the
wrong enabled state until
+ * the first change.
+ */
+ @Test
+ void aListeningPluginIsToldItsWidgetsAreFilled() {
+ GuiCompositeWidgets widgets = mock(GuiCompositeWidgets.class);
+ IGuiPluginCompositeWidgetsListener plugin =
mock(IGuiPluginCompositeWidgetsListener.class);
+
+ ConfigPluginOptionsTab.attachWidgetsListener(widgets, plugin);
+
+ InOrder order = inOrder(widgets, plugin);
+ order.verify(widgets).setWidgetsListener(plugin);
+ order.verify(plugin).widgetsPopulated(widgets);
+ }
+
+ @Test
+ void aPluginThatDoesNotListenIsLeftAlone() {
+ GuiCompositeWidgets widgets = mock(GuiCompositeWidgets.class);
+
+ ConfigPluginOptionsTab.attachWidgetsListener(widgets, new Object());
+
+ verify(widgets, never()).setWidgetsListener(any());
+ }
+}
