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 2d0fb90192 Fixes #8734 : Save only the changed rule from the Lint
Rules Manager and let it edit every rule (#8738)
2d0fb90192 is described below
commit 2d0fb901920330a4a90470ad7ecab4d87b8ae861
Author: Bart Maertens <[email protected]>
AuthorDate: Sat Oct 3 10:33:59 2026 +0200
Fixes #8734 : Save only the changed rule from the Lint Rules Manager and
let it edit every rule (#8738)
---
.../modules/ROOT/pages/linting/lint-rules.adoc | 3 +-
.../org/apache/hop/lint/LintPolicyYamlWriter.java | 290 ++++++++++++++++++++-
.../org/apache/hop/lint/LinterConfigPlugin.java | 49 ++--
.../org/apache/hop/lint/RuleBuilderDialog.java | 133 ++++++----
.../org/apache/hop/lint/RuleManagerDialog.java | 77 ++++--
.../java/org/apache/hop/lint/RuleTargetFields.java | 51 +++-
.../hop/lint/registry/ProjectLintYamlExporter.java | 60 +++--
.../hop/lint/messages/messages_en_US.properties | 3 +
.../apache/hop/lint/LintRuleYamlWriterTest.java | 255 ++++++++++++++++++
.../hop/lint/RuleBuilderDialogNativeRuleTest.java | 69 +++++
.../org/apache/hop/lint/RuleEditorChoicesTest.java | 133 ++++++++++
11 files changed, 1010 insertions(+), 113 deletions(-)
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 523eb6366b..7486c4fc12 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
@@ -198,7 +198,7 @@ A rule with neither block behaves exactly as before, so
nothing that was written
[NOTE]
====
-The rule manager in Hop Gui edits one condition, so it shows a composed rule's
clauses but leaves them alone: name, severity and enabled can be changed there,
the clauses are edited in `hop-lint.yml`.
+The rule editor in Hop Gui lists a composed rule's clauses and edits them one
at a time; *Add clause* and *Remove clause* change the list.
====
== Narrowing a rule to one kind of transform
@@ -404,6 +404,7 @@ rules:
----
*Tools -> Lint -> Manage Custom Rules* writes this file for you.
+It changes only the entry of the rule you added, edited, switched or deleted:
the other rules, the `exclude` and `suppress` sections and your comments stay
as you wrote them.
A parse error in `hop-lint.yml` is reported rather than ignored, so that a
typo cannot leave you linting with the defaults while believing otherwise.
diff --git
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintPolicyYamlWriter.java
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintPolicyYamlWriter.java
index 7aa8f69433..d5bbd45925 100644
---
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintPolicyYamlWriter.java
+++
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintPolicyYamlWriter.java
@@ -22,13 +22,17 @@ import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
+import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
import org.apache.hop.core.util.Utils;
+import org.yaml.snakeyaml.DumperOptions;
import org.yaml.snakeyaml.Yaml;
/**
- * Adds an exclusion or a suppression to a project's {@code hop-lint.yml} from
the user interface.
+ * Adds an exclusion, a suppression or a rule to a project's {@code
hop-lint.yml} from the user
+ * interface.
*
* <p>The file is edited as text rather than parsed and rewritten. A project's
lint configuration is
* meant to be read and hand-edited — the documentation says as much — and
round-tripping it through
@@ -44,6 +48,8 @@ public final class LintPolicyYamlWriter {
private static final String EXCLUDE_KEY = "exclude";
private static final String SUPPRESS_KEY = "suppress";
+ private static final String RULES_KEY = "rules";
+ private static final int DEFAULT_RULE_INDENT = 2;
private LintPolicyYamlWriter() {}
@@ -214,6 +220,288 @@ public final class LintPolicyYamlWriter {
return removed;
}
+ /**
+ * Write one rule's entry under {@code rules:}, in place of the entry that
is there.
+ *
+ * <p>The rule manager used to write the whole file from its list of rules,
which deleted the
+ * {@code exclude} and {@code suppress} sections with their reasons, every
comment, and the order
+ * the rules were written in, whatever rule was changed. Only the lines of
this one rule are
+ * replaced now, and the save is refused unless everything else in the file
reads back the same.
+ *
+ * @param ruleId the rule's key under {@code rules:}, matched ignoring case
+ * @param entry the keys to write for it; null or empty removes the entry
+ */
+ public static void putRule(Path yamlFile, String ruleId, Map<String, Object>
entry)
+ throws IOException {
+ if (isBlank(ruleId)) {
+ throw new IOException("A rule needs an id");
+ }
+ String original =
+ Files.exists(yamlFile) ? Files.readString(yamlFile,
StandardCharsets.UTF_8) : "";
+ String updated = replaceRule(original, ruleId, entry);
+ if (updated.equals(original)) {
+ return;
+ }
+ if (yamlFile.getParent() != null) {
+ Files.createDirectories(yamlFile.getParent());
+ }
+ Files.writeString(yamlFile, updated, StandardCharsets.UTF_8);
+ }
+
+ /**
+ * Remove one rule's entry from {@code rules:}, leaving the rest of the file
as it was.
+ *
+ * @return true when an entry was removed
+ */
+ public static boolean removeRule(Path yamlFile, String ruleId) throws
IOException {
+ if (!Files.exists(yamlFile) || isBlank(ruleId)) {
+ return false;
+ }
+ String original = Files.readString(yamlFile, StandardCharsets.UTF_8);
+ String updated = replaceRule(original, ruleId, null);
+ if (updated.equals(original)) {
+ return false;
+ }
+ Files.writeString(yamlFile, updated, StandardCharsets.UTF_8);
+ return true;
+ }
+
+ /** The file with this rule's entry replaced, added or, for a null or empty
entry, removed. */
+ static String replaceRule(String original, String ruleId, Map<String,
Object> entry)
+ throws IOException {
+ Map<?, ?> before = loadMapping(original);
+ boolean remove = entry == null || entry.isEmpty();
+
+ List<String> lines = new ArrayList<>(List.of(original.split("\n", -1)));
+ int keyLine = indexOfRulesKey(lines);
+
+ String updated;
+ if (keyLine < 0) {
+ if (remove) {
+ return original;
+ }
+ updated = insert(original, RULES_KEY, renderRule(ruleId, entry,
DEFAULT_RULE_INDENT));
+ } else {
+ int blockEnd = endOfBlock(lines, keyLine);
+ int indent = childIndent(lines, keyLine + 1, blockEnd);
+ int start = indexOfRule(lines, keyLine + 1, blockEnd, indent, ruleId);
+
+ List<String> rendered = remove ? List.of() : renderRule(ruleId, entry,
indent);
+ if (start < 0) {
+ if (remove) {
+ return original;
+ }
+ lines.addAll(blockEnd, rendered);
+ } else {
+ int end = endOfRule(lines, start, blockEnd, indent);
+ lines.subList(start, end).clear();
+ lines.addAll(start, rendered);
+ // A rules: key with nothing under it reads as an oversight, so the
last entry takes the
+ // key with it, as the last suppression does.
+ if (remove && !hasContent(lines, keyLine + 1, endOfBlock(lines,
keyLine))) {
+ lines.remove(keyLine);
+ }
+ }
+ updated = String.join("\n", lines);
+ }
+
+ verifyRuleEdit(before, loadMapping(updated), ruleId, remove ? null :
entry);
+ return updated;
+ }
+
+ /**
+ * The line holding {@code rules:}, or -1. An empty inline {@code rules: {}}
is turned into a
+ * block key so an entry can go under it; any other inline form is left for
the user to edit.
+ */
+ private static int indexOfRulesKey(List<String> lines) throws IOException {
+ for (int i = 0; i < lines.size(); i++) {
+ String line = lines.get(i);
+ if (!line.startsWith(RULES_KEY + ":")) {
+ continue;
+ }
+ String rest = stripComment(line.substring(RULES_KEY.length() +
1)).trim();
+ if (rest.isEmpty()) {
+ return i;
+ }
+ if (rest.equals("{}")) {
+ lines.set(i, RULES_KEY + ":");
+ return i;
+ }
+ throw new IOException("hop-lint.yml writes its rules inline; edit the
rule there by hand");
+ }
+ return -1;
+ }
+
+ /** The indentation of the rule keys under {@code rules:}, as the file
already uses it. */
+ private static int childIndent(List<String> lines, int from, int to) {
+ for (int i = from; i < to; i++) {
+ if (isContent(lines.get(i))) {
+ return indentOf(lines.get(i));
+ }
+ }
+ return DEFAULT_RULE_INDENT;
+ }
+
+ private static int indexOfRule(List<String> lines, int from, int to, int
indent, String ruleId) {
+ for (int i = from; i < to; i++) {
+ String line = lines.get(i);
+ if (!isContent(line) || indentOf(line) != indent) {
+ continue;
+ }
+ String trimmed = line.trim();
+ int colon = keyEnd(trimmed);
+ if (colon > 0 && ruleId.equalsIgnoreCase(unquoteKey(trimmed.substring(0,
colon)))) {
+ return i;
+ }
+ }
+ return -1;
+ }
+
+ /**
+ * Where a rule's lines end: at the next key of the same depth. Blank lines
and comments just
+ * above that key describe it, not this rule, so they stay.
+ */
+ private static int endOfRule(List<String> lines, int start, int blockEnd,
int indent) {
+ int end = blockEnd;
+ for (int i = start + 1; i < blockEnd; i++) {
+ if (isContent(lines.get(i)) && indentOf(lines.get(i)) <= indent) {
+ end = i;
+ break;
+ }
+ }
+ while (end - 1 > start) {
+ String line = lines.get(end - 1);
+ if (line.isBlank() || (line.trim().startsWith("#") && indentOf(line) <=
indent)) {
+ end--;
+ } else {
+ break;
+ }
+ }
+ return end;
+ }
+
+ private static List<String> renderRule(String ruleId, Map<String, Object>
entry, int indent) {
+ DumperOptions options = new DumperOptions();
+ options.setDefaultFlowStyle(DumperOptions.FlowStyle.BLOCK);
+ options.setIndent(Math.max(2, Math.min(indent, 10)));
+ Map<String, Object> wrapper = new LinkedHashMap<>();
+ wrapper.put(ruleId, entry);
+ String prefix = " ".repeat(indent);
+ List<String> rendered = new ArrayList<>();
+ for (String line : new Yaml(options).dump(wrapper).split("\n")) {
+ if (!line.isEmpty()) {
+ rendered.add(prefix + line);
+ }
+ }
+ return rendered;
+ }
+
+ /**
+ * Refuse to save unless the rule reads back as written and nothing else in
the file changed. Text
+ * editing keeps the user's file intact; this makes sure it also kept it
correct.
+ */
+ private static void verifyRuleEdit(
+ Map<?, ?> before, Map<?, ?> after, String ruleId, Map<String, Object>
entry)
+ throws IOException {
+ for (Object key : union(before.keySet(), after.keySet())) {
+ if (!RULES_KEY.equals(key) && !Objects.equals(before.get(key),
after.get(key))) {
+ throw new IOException("Editing a rule would have changed '" + key + "'
in hop-lint.yml");
+ }
+ }
+ Map<?, ?> rulesBefore = asMap(before.get(RULES_KEY));
+ Map<?, ?> rulesAfter = asMap(after.get(RULES_KEY));
+ for (Object key : union(rulesBefore.keySet(), rulesAfter.keySet())) {
+ if (String.valueOf(key).equalsIgnoreCase(ruleId)) {
+ continue;
+ }
+ if (!Objects.equals(rulesBefore.get(key), rulesAfter.get(key))) {
+ throw new IOException(
+ "Editing rule " + ruleId + " would have changed rule " + key + "
in hop-lint.yml");
+ }
+ }
+ Object written = rulesAfter.get(ruleId);
+ Object expected = entry == null ? null : new Yaml().load(new
Yaml().dump(entry));
+ if (!Objects.equals(expected, written)) {
+ throw new IOException("Rule " + ruleId + " did not survive the edit,
change it by hand");
+ }
+ }
+
+ private static Map<?, ?> loadMapping(String text) throws IOException {
+ Object parsed;
+ try {
+ parsed = new Yaml().load(text);
+ } catch (Exception e) {
+ throw new IOException("hop-lint.yml cannot be read: " + e.getMessage(),
e);
+ }
+ if (parsed == null) {
+ return Map.of();
+ }
+ if (!(parsed instanceof Map)) {
+ throw new IOException("hop-lint.yml is not a YAML mapping, change the
rule by hand");
+ }
+ return (Map<?, ?>) parsed;
+ }
+
+ private static Map<?, ?> asMap(Object value) {
+ return value instanceof Map<?, ?> map ? map : Map.of();
+ }
+
+ private static List<Object> union(java.util.Collection<?> first,
java.util.Collection<?> second) {
+ List<Object> keys = new ArrayList<>(first);
+ for (Object key : second) {
+ if (!keys.contains(key)) {
+ keys.add(key);
+ }
+ }
+ return keys;
+ }
+
+ private static boolean hasContent(List<String> lines, int from, int to) {
+ for (int i = from; i < to; i++) {
+ if (isContent(lines.get(i))) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ private static boolean isContent(String line) {
+ String trimmed = line.trim();
+ return !trimmed.isEmpty() && !trimmed.startsWith("#");
+ }
+
+ private static int indentOf(String line) {
+ int indent = 0;
+ while (indent < line.length() && line.charAt(indent) == ' ') {
+ indent++;
+ }
+ return indent;
+ }
+
+ /** The position of the colon ending a mapping key, skipping one inside a
quoted key. */
+ private static int keyEnd(String trimmed) {
+ if (trimmed.startsWith("\"") || trimmed.startsWith("'")) {
+ int close = trimmed.indexOf(trimmed.charAt(0), 1);
+ return close < 0 ? -1 : trimmed.indexOf(':', close);
+ }
+ return trimmed.indexOf(':');
+ }
+
+ private static String unquoteKey(String key) {
+ String trimmed = key.trim();
+ if (trimmed.length() >= 2
+ && (trimmed.startsWith("\"") && trimmed.endsWith("\"")
+ || trimmed.startsWith("'") && trimmed.endsWith("'"))) {
+ return trimmed.substring(1, trimmed.length() - 1);
+ }
+ return trimmed;
+ }
+
+ private static String stripComment(String value) {
+ int hash = value.indexOf(" #");
+ return hash < 0 ? value : value.substring(0, hash);
+ }
+
/**
* Split a block into its list items, each starting at a line whose first
token is "-".
*
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 7c41c310ee..01b99b4928 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
@@ -17,6 +17,9 @@
package org.apache.hop.lint;
import java.io.File;
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
@@ -406,9 +409,6 @@ public class LinterConfigPlugin implements IConfigOptions,
IGuiPluginCompositeWi
if (written.isEmpty()) {
return false;
}
- if (!Utils.isEmpty(configFilePath)) {
- saveConfiguration();
- }
log.logBasic("Linter configuration updated");
return true;
}
@@ -569,32 +569,29 @@ public class LinterConfigPlugin implements
IConfigOptions, IGuiPluginCompositeWi
return exportToYaml(getCustomRules());
}
- /** Save configuration to the specified file path */
- public boolean saveConfiguration() {
- return saveProjectRules(getCustomRules());
+ /**
+ * Write one rule's state to the project's hop-lint.yml, leaving every other
line of it alone.
+ *
+ * @param rule the rule as the rule manager now has it
+ * @param previousId the id it was saved under before, when that differs;
that entry is removed
+ * @throws IOException when the file cannot be changed safely; nothing is
written then
+ */
+ public void saveProjectRule(CustomLintRule rule, String previousId) throws
IOException {
+ Path path = Paths.get(resolveProjectConfigPath());
+ String ruleId = rule.generateRuleId();
+ if (!Utils.isEmpty(previousId) && !previousId.equalsIgnoreCase(ruleId)) {
+ LintPolicyYamlWriter.removeRule(path, previousId);
+ }
+ LintPolicyYamlWriter.putRule(path, ruleId,
ProjectLintYamlExporter.entryFor(rule));
+ log.logBasic("Saved lint rule " + ruleId + " to " + path);
}
- /** Save project hop-lint.yml from the rule manager's desired effective
state. */
- public boolean saveProjectRules(List<CustomLintRule> desiredRules) {
- try {
- String yamlContent = exportToYaml(desiredRules);
- if (yamlContent == null) {
- return false;
- }
- String savePath = resolveProjectConfigPath();
- File parentDir = new File(savePath).getParentFile();
- if (parentDir != null && !parentDir.exists()) {
- parentDir.mkdirs();
- }
- java.nio.file.Files.write(
- java.nio.file.Paths.get(savePath),
- yamlContent.getBytes(java.nio.charset.StandardCharsets.UTF_8));
- log.logBasic("Linter configuration saved to: " + savePath);
- return true;
- } catch (Exception e) {
- log.logError("Error saving linter configuration: " + e.getMessage(), e);
+ /** Remove a project rule's entry from the project's hop-lint.yml. */
+ public void removeProjectRule(String ruleId) throws IOException {
+ Path path = Paths.get(resolveProjectConfigPath());
+ if (LintPolicyYamlWriter.removeRule(path, ruleId)) {
+ log.logBasic("Removed lint rule " + ruleId + " from " + path);
}
- return false;
}
private String resolveProjectConfigPath() {
diff --git
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/RuleBuilderDialog.java
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/RuleBuilderDialog.java
index b433039eb4..cb864aa625 100644
--- a/plugins/misc/lint/src/main/java/org/apache/hop/lint/RuleBuilderDialog.java
+++ b/plugins/misc/lint/src/main/java/org/apache/hop/lint/RuleBuilderDialog.java
@@ -28,6 +28,7 @@ import org.eclipse.swt.layout.FormData;
import org.eclipse.swt.layout.FormLayout;
import org.eclipse.swt.widgets.Button;
import org.eclipse.swt.widgets.Combo;
+import org.eclipse.swt.widgets.Control;
import org.eclipse.swt.widgets.Dialog;
import org.eclipse.swt.widgets.Display;
import org.eclipse.swt.widgets.Label;
@@ -60,6 +61,7 @@ public class RuleBuilderDialog extends Dialog {
private Button enabledCheck;
private Combo combinatorCombo;
private Table clauseTable;
+ private Button addClauseButton;
private Button removeClauseButton;
/**
@@ -67,6 +69,9 @@ public class RuleBuilderDialog extends Dialog {
*/
private final List<RuleClause> editingClauses = new ArrayList<>();
+ /** What the condition combo offers, in the order it shows them. */
+ private List<RuleCondition> conditionChoices = new ArrayList<>();
+
/** Guards the widget listeners while the widgets are being loaded from a
clause. */
private boolean loadingClause = false;
@@ -161,7 +166,7 @@ public class RuleBuilderDialog extends Dialog {
new SelectionAdapter() {
@Override
public void widgetSelected(SelectionEvent e) {
- updateFieldCombo();
+ updateFieldCombo(null);
}
});
FormData targetData = new FormData();
@@ -219,7 +224,7 @@ public class RuleBuilderDialog extends Dialog {
clauseTableData.height = 90;
clauseTable.setLayoutData(clauseTableData);
- Button addClauseButton = new Button(shell, SWT.PUSH);
+ addClauseButton = new Button(shell, SWT.PUSH);
addClauseButton.setText(BaseMessages.getString(PKG,
"RuleBuilderDialog.Button.AddClause"));
addClauseButton.addSelectionListener(
new SelectionAdapter() {
@@ -262,7 +267,7 @@ public class RuleBuilderDialog extends Dialog {
new SelectionAdapter() {
@Override
public void widgetSelected(SelectionEvent e) {
- updateConditionCombo();
+ updateConditionCombo(null);
captureWidgetsIntoSelectedClause();
}
});
@@ -323,8 +328,10 @@ public class RuleBuilderDialog extends Dialog {
severityLabel.setLayoutData(severityLabelData);
severityCombo = new Combo(shell, SWT.DROP_DOWN | SWT.READ_ONLY);
- severityCombo.setItems(new String[] {"ERROR", "WARNING"});
- severityCombo.select(1); // Default to WARNING
+ for (LintSeverity.Level level : LintSeverity.Level.values()) {
+ severityCombo.add(level.name());
+ }
+ severityCombo.select(LintSeverity.Level.WARNING.ordinal());
FormData severityData = new FormData();
severityData.left = new FormAttachment(severityLabel, margin);
severityData.right = new FormAttachment(100, -margin);
@@ -383,54 +390,89 @@ public class RuleBuilderDialog extends Dialog {
}
if (rule.getTarget() != null) {
targetCombo.select(rule.getTarget().ordinal());
- updateFieldCombo();
+ updateFieldCombo(null);
}
+ // Selected by index: setText does nothing on a read-only combo, which
left WARNING showing for
+ // an INFO rule and saved it as WARNING.
if (rule.getSeverity() != null) {
- severityCombo.setText(rule.getSeverity());
+ int severityIndex =
severityCombo.indexOf(rule.getSeverity().trim().toUpperCase());
+ if (severityIndex >= 0) {
+ severityCombo.select(severityIndex);
+ }
}
enabledCheck.setSelection(rule.isEnabled());
loadClausesFromRule();
+
+ // A native rule, such as HOP-CHECK, says how Hop's own verify remarks are
reported. It has no
+ // target, field or condition, and the editor refused to save it, even a
severity change.
+ if (rule.isNativeVerify()) {
+ for (Control control :
+ new Control[] {
+ targetCombo,
+ combinatorCombo,
+ clauseTable,
+ addClauseButton,
+ removeClauseButton,
+ fieldCombo,
+ conditionCombo,
+ valueText
+ }) {
+ control.setEnabled(false);
+ }
+ }
}
- private void updateFieldCombo() {
+ /**
+ * @param currentField the field the clause being shown reads, kept in the
list even when the
+ * editor would not have offered it
+ */
+ private void updateFieldCombo(String currentField) {
fieldCombo.removeAll();
conditionCombo.removeAll();
+ conditionChoices = new ArrayList<>();
int targetIndex = targetCombo.getSelectionIndex();
if (targetIndex >= 0) {
RuleTarget target = RuleTarget.values()[targetIndex];
- List<String> fields = RuleTargetFields.getFieldsForTarget(target);
- for (String field : fields) {
+ for (String field : RuleTargetFields.getFieldChoices(target,
currentField)) {
fieldCombo.add(field);
}
}
}
- private void updateConditionCombo() {
+ /**
+ * @param currentCondition the condition the clause being shown uses, kept
in the list even when
+ * the editor would not have offered it for this field
+ */
+ private void updateConditionCombo(RuleCondition currentCondition) {
conditionCombo.removeAll();
+ conditionChoices = new ArrayList<>();
String selectedField = fieldCombo.getText();
if (!Utils.isEmpty(selectedField)) {
- List<RuleCondition> conditions =
RuleTargetFields.getCompatibleConditions(selectedField);
- for (RuleCondition condition : conditions) {
+ conditionChoices = RuleTargetFields.getConditionChoices(selectedField,
currentCondition);
+ for (RuleCondition condition : conditionChoices) {
conditionCombo.add(condition.getDisplayName());
}
}
}
- private void updateValueField() {
+ private RuleCondition selectedCondition() {
int conditionIndex = conditionCombo.getSelectionIndex();
- if (conditionIndex >= 0) {
- String selectedField = fieldCombo.getText();
- List<RuleCondition> conditions =
RuleTargetFields.getCompatibleConditions(selectedField);
- if (conditionIndex < conditions.size()) {
- RuleCondition condition = conditions.get(conditionIndex);
- valueLabel.setVisible(condition.requiresValue());
- valueText.setVisible(condition.requiresValue());
-
- if (condition.requiresValue()) {
- valueText.setToolTipText(condition.getDescription());
- }
+ if (conditionIndex < 0 || conditionIndex >= conditionChoices.size()) {
+ return null;
+ }
+ return conditionChoices.get(conditionIndex);
+ }
+
+ private void updateValueField() {
+ RuleCondition condition = selectedCondition();
+ if (condition != null) {
+ valueLabel.setVisible(condition.requiresValue());
+ valueText.setVisible(condition.requiresValue());
+
+ if (condition.requiresValue()) {
+ valueText.setToolTipText(condition.getDescription());
}
}
shell.layout(true, true);
@@ -449,6 +491,9 @@ public class RuleBuilderDialog extends Dialog {
showError("Rule name is required");
return false;
}
+ if (rule.isNativeVerify()) {
+ return true;
+ }
if (targetCombo.getSelectionIndex() < 0) {
showError("Target type must be selected");
return false;
@@ -463,27 +508,25 @@ public class RuleBuilderDialog extends Dialog {
}
// Check if condition requires a value
- String selectedField = fieldCombo.getText();
- List<RuleCondition> conditions =
RuleTargetFields.getCompatibleConditions(selectedField);
- int conditionIndex = conditionCombo.getSelectionIndex();
- if (conditionIndex < conditions.size()) {
- RuleCondition condition = conditions.get(conditionIndex);
- if (condition.requiresValue() && Utils.isEmpty(valueText.getText())) {
- showError("Value is required for this condition");
- return false;
- }
+ RuleCondition condition = selectedCondition();
+ if (condition != null && condition.requiresValue() &&
Utils.isEmpty(valueText.getText())) {
+ showError("Value is required for this condition");
+ return false;
}
return true;
}
private void saveRule() {
- captureWidgetsIntoSelectedClause();
-
rule.setName(nameText.getText());
rule.setDescription(descriptionText.getText());
rule.setSeverity(severityCombo.getText());
rule.setEnabled(enabledCheck.getSelection());
+ if (rule.isNativeVerify()) {
+ return;
+ }
+
+ captureWidgetsIntoSelectedClause();
rule.setTarget(RuleTarget.values()[targetCombo.getSelectionIndex()]);
rule.setCombinator(
combinatorCombo.getSelectionIndex() == 1 ? RuleCombinator.ANY_OF :
RuleCombinator.ALL_OF);
@@ -543,21 +586,16 @@ public class RuleBuilderDialog extends Dialog {
RuleClause clause = editingClauses.get(index);
loadingClause = true;
try {
- updateFieldCombo();
+ updateFieldCombo(clause.getTargetField());
if (clause.getTargetField() != null) {
int fieldIndex = fieldCombo.indexOf(clause.getTargetField());
if (fieldIndex >= 0) {
fieldCombo.select(fieldIndex);
}
}
- updateConditionCombo();
+ updateConditionCombo(clause.getCondition());
if (clause.getCondition() != null) {
- for (int i = 0; i < conditionCombo.getItemCount(); i++) {
- if
(conditionCombo.getItem(i).equals(clause.getCondition().getDisplayName())) {
- conditionCombo.select(i);
- break;
- }
- }
+ conditionCombo.select(conditionChoices.indexOf(clause.getCondition()));
}
updateValueField();
valueText.setText(clause.getConditionValue() == null ? "" :
clause.getConditionValue());
@@ -577,10 +615,9 @@ public class RuleBuilderDialog extends Dialog {
}
RuleClause clause = editingClauses.get(index);
clause.setTargetField(fieldCombo.getText());
- List<RuleCondition> conditions =
RuleTargetFields.getCompatibleConditions(fieldCombo.getText());
- int conditionIndex = conditionCombo.getSelectionIndex();
- if (conditionIndex >= 0 && conditionIndex < conditions.size()) {
- clause.setCondition(conditions.get(conditionIndex));
+ RuleCondition condition = selectedCondition();
+ if (condition != null) {
+ clause.setCondition(condition);
}
clause.setConditionValue(valueText.getText());
refreshClauseTable();
diff --git
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/RuleManagerDialog.java
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/RuleManagerDialog.java
index 8855c6788b..101b8a3b59 100644
--- a/plugins/misc/lint/src/main/java/org/apache/hop/lint/RuleManagerDialog.java
+++ b/plugins/misc/lint/src/main/java/org/apache/hop/lint/RuleManagerDialog.java
@@ -74,7 +74,7 @@ public class RuleManagerDialog extends Dialog {
// at the default size. The table scrolls horizontally if the user makes
it narrower.
shell.setSize(1220, 620);
shell.setLocation(
- parent.getLocation().x + (parent.getSize().x - 900) / 2,
+ parent.getLocation().x + (parent.getSize().x - 1220) / 2,
parent.getLocation().y + (parent.getSize().y - 620) / 2);
shell.open();
@@ -133,9 +133,15 @@ public class RuleManagerDialog extends Dialog {
}
});
+ // Findings, hop-lint.yml and the CLI all name a rule by its id, and two
rules can share the
+ // start of a long name, so the id comes first.
+ TableColumn idColumn = new TableColumn(rulesTable, SWT.LEFT);
+ idColumn.setText(BaseMessages.getString(PKG,
"RuleManagerDialog.Column.RuleId"));
+ idColumn.setWidth(110);
+
TableColumn nameColumn = new TableColumn(rulesTable, SWT.LEFT);
nameColumn.setText(BaseMessages.getString(PKG,
"RuleManagerDialog.Column.RuleName"));
- nameColumn.setWidth(180);
+ nameColumn.setWidth(260);
TableColumn sourceColumn = new TableColumn(rulesTable, SWT.LEFT);
sourceColumn.setText(BaseMessages.getString(PKG,
"RuleManagerDialog.Column.Source"));
@@ -277,24 +283,25 @@ public class RuleManagerDialog extends Dialog {
for (CustomLintRule rule : rules) {
TableItem item = new TableItem(rulesTable, SWT.NONE);
- item.setText(0, rule.getName() != null ? rule.getName() : "");
- item.setText(1, rule.getPackOwner() != null ?
rule.getPackOwner().getDisplayName() : "");
- item.setText(2, rule.getTarget() != null ?
rule.getTarget().getDisplayName() : "");
- item.setText(3, rule.getTargetField() != null ? rule.getTargetField() :
"");
+ item.setText(0, rule.getTarget() != null ? rule.generateRuleId() :
nullToEmpty(rule.getId()));
+ item.setText(1, rule.getName() != null ? rule.getName() : "");
+ item.setText(2, rule.getPackOwner() != null ?
rule.getPackOwner().getDisplayName() : "");
+ item.setText(3, rule.getTarget() != null ?
rule.getTarget().getDisplayName() : "");
+ item.setText(4, rule.getTargetField() != null ? rule.getTargetField() :
"");
// A composed rule checks several things, and showing only its first
clause made it
// indistinguishable from a rule that checks one. Say so in the columns
that would otherwise
// be a half-truth.
if (rule.isComposed()) {
item.setText(
- 3, rule.getClauses().size() + " fields (" +
rule.getCombinator().getYamlKey() + ")");
- item.setText(4, rule.getCombinator() == RuleCombinator.ALL_OF ? "All
of" : "Any of");
- item.setText(5, "");
+ 4, rule.getClauses().size() + " fields (" +
rule.getCombinator().getYamlKey() + ")");
+ item.setText(5, rule.getCombinator() == RuleCombinator.ALL_OF ? "All
of" : "Any of");
+ item.setText(6, "");
} else {
- item.setText(4, rule.getCondition() != null ?
rule.getCondition().getDisplayName() : "");
- item.setText(5, rule.getConditionValue() != null ?
rule.getConditionValue() : "");
+ item.setText(5, rule.getCondition() != null ?
rule.getCondition().getDisplayName() : "");
+ item.setText(6, rule.getConditionValue() != null ?
rule.getConditionValue() : "");
}
- item.setText(6, rule.getSeverity() != null ? rule.getSeverity() :
"WARNING");
- item.setText(7, rule.isEnabled() ? "✓" : "✗");
+ item.setText(7, rule.getSeverity() != null ? rule.getSeverity() :
"WARNING");
+ item.setText(8, rule.isEnabled() ? "✓" : "✗");
item.setData(rule);
}
}
@@ -325,9 +332,12 @@ public class RuleManagerDialog extends Dialog {
if (newRule != null) {
newRule.setPackId(RulePackIds.PROJECT);
newRule.setPackOwner(RulePackOwner.PROJECT);
+ // Fixed now: without an id of its own the rule is named after a hash of
its field and
+ // condition, so editing either would save it under a new id and leave
the old entry behind.
+ newRule.setId(newRule.generateRuleId());
rules.add(newRule);
populateTable();
- saveConfiguration();
+ saveRule(newRule, null);
log.logBasic("Added project rule: " + newRule.getName());
}
}
@@ -342,11 +352,12 @@ public class RuleManagerDialog extends Dialog {
// the project's hop-lint.yml, as an override when it only tunes the rule
and as a full rule
// definition when it redefines it. That keeps the pack upgradeable and
the decision with the
// project.
+ String previousId = rule.generateRuleId();
RuleBuilderDialog dialog = new RuleBuilderDialog(shell, rule);
CustomLintRule editedRule = dialog.open();
if (editedRule != null) {
populateTable();
- saveConfiguration();
+ saveRule(editedRule, previousId);
log.logBasic("Edited rule: " + rule.getName());
}
}
@@ -372,7 +383,11 @@ public class RuleManagerDialog extends Dialog {
rules.remove(rule);
populateTable();
updateButtonState();
- saveConfiguration();
+ try {
+
LinterConfigPlugin.getInstance().removeProjectRule(rule.generateRuleId());
+ } catch (Exception e) {
+ showSaveError(rule, e);
+ }
log.logBasic("Deleted project rule: " + rule.getName());
}
}
@@ -388,7 +403,7 @@ public class RuleManagerDialog extends Dialog {
populateTable();
rulesTable.setSelection(findTableIndex(rule));
updateButtonState();
- saveConfiguration();
+ saveRule(rule, null);
log.logBasic(
"Toggled rule: " + rule.getName() + " to " + (rule.isEnabled() ?
"enabled" : "disabled"));
}
@@ -423,12 +438,32 @@ public class RuleManagerDialog extends Dialog {
messageBox.open();
}
- private void saveConfiguration() {
+ /**
+ * Write the one rule that changed. The rest of hop-lint.yml — other rules,
exclusions,
+ * suppressions and comments — stays as it is.
+ */
+ private void saveRule(CustomLintRule rule, String previousId) {
try {
- LinterConfigPlugin configPlugin = LinterConfigPlugin.getInstance();
- configPlugin.saveProjectRules(new java.util.ArrayList<>(rules));
+ LinterConfigPlugin.getInstance().saveProjectRule(rule, previousId);
} catch (Exception e) {
- log.logError("Error saving custom rules configuration: " +
e.getMessage(), e);
+ showSaveError(rule, e);
}
}
+
+ /** A save that failed used to be logged only, so the change looked saved
and was not. */
+ private void showSaveError(CustomLintRule rule, Exception e) {
+ log.logError("Error saving lint rule " + rule.generateRuleId() + ": " +
e.getMessage(), e);
+ MessageBox messageBox = new MessageBox(shell, SWT.ICON_ERROR | SWT.OK);
+ messageBox.setText(BaseMessages.getString(PKG,
"RuleManagerDialog.Dialog.SaveFailed.Title"));
+ messageBox.setMessage(
+ BaseMessages.getString(
+ PKG, "RuleManagerDialog.Dialog.SaveFailed.Message",
rule.generateRuleId())
+ + "\n\n"
+ + e.getMessage());
+ messageBox.open();
+ }
+
+ private static String nullToEmpty(String value) {
+ return value == null ? "" : value;
+ }
}
diff --git
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/RuleTargetFields.java
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/RuleTargetFields.java
index 2b2df4c07d..4c7185f444 100644
--- a/plugins/misc/lint/src/main/java/org/apache/hop/lint/RuleTargetFields.java
+++ b/plugins/misc/lint/src/main/java/org/apache/hop/lint/RuleTargetFields.java
@@ -16,6 +16,7 @@
*/
package org.apache.hop.lint;
+import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
@@ -109,6 +110,8 @@ public class RuleTargetFields {
"targetTransforms",
"isStart",
"isDummy",
+ "isOrphaned",
+ "isBlockingTransform",
"hasDefaultName",
"password",
"secret",
@@ -128,6 +131,7 @@ public class RuleTargetFields {
"errorHandling",
"targetActions",
"isStart",
+ "isOrphaned",
"hasDefaultName",
"password",
"secret",
@@ -199,7 +203,7 @@ public class RuleTargetFields {
// Numeric fields
return Arrays.asList(
RuleCondition.MAX_VALUE, RuleCondition.MIN_VALUE,
RuleCondition.EXACT_VALUE);
- } else if (field.startsWith("has")
+ } else if (isFlagName(field)
|| field.equals("enabled")
|| field.equals("distributes")
|| field.equals("unconditional")
@@ -230,4 +234,49 @@ public class RuleTargetFields {
RuleCondition.ENDS_WITH);
}
}
+
+ /**
+ * {@code hasNotes}, {@code isDummy}: a yes-or-no question, but not {@code
issuer} or {@code
+ * hash}.
+ */
+ private static boolean isFlagName(String field) {
+ for (String prefix : new String[] {"has", "is"}) {
+ if (field.length() > prefix.length()
+ && field.startsWith(prefix)
+ && Character.isUpperCase(field.charAt(prefix.length()))) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ /**
+ * The fields the rule editor offers, with the one the rule already reads
among them.
+ *
+ * <p>A rule may read a field the list does not know: a transform's own
setting such as a Table
+ * Input's {@code sql}, or any field of a metadata object. Leaving it out
opened such a rule with
+ * no field selected, and the editor then refused to save it at all, even
for a severity change.
+ */
+ public static List<String> getFieldChoices(RuleTarget target, String
currentField) {
+ List<String> fields = new ArrayList<>(getFieldsForTarget(target));
+ if (currentField != null && !currentField.isEmpty() &&
!fields.contains(currentField)) {
+ fields.add(currentField);
+ }
+ return fields;
+ }
+
+ /**
+ * The conditions the rule editor offers for a field, with the one the rule
already uses among
+ * them, for the same reason as {@link #getFieldChoices}: a rule from a pack
is not wrong just
+ * because the editor would not have suggested its condition.
+ */
+ public static List<RuleCondition> getConditionChoices(
+ String field, RuleCondition currentCondition) {
+ List<RuleCondition> conditions =
+ new ArrayList<>(getCompatibleConditions(field == null ? "" : field));
+ if (currentCondition != null && !conditions.contains(currentCondition)) {
+ conditions.add(currentCondition);
+ }
+ return conditions;
+ }
}
diff --git
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/registry/ProjectLintYamlExporter.java
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/registry/ProjectLintYamlExporter.java
index 3bbd481f93..443adcb106 100644
---
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/registry/ProjectLintYamlExporter.java
+++
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/registry/ProjectLintYamlExporter.java
@@ -34,23 +34,13 @@ public final class ProjectLintYamlExporter {
public static String export(List<CustomLintRule> desiredRules) {
try {
- Map<String, CustomLintRule> packDefaults = new LinkedHashMap<>();
- for (CustomLintRule rule :
RuleRegistry.getInstance().resolve(null).getRules()) {
- packDefaults.put(rule.generateRuleId(), rule);
- }
+ Map<String, CustomLintRule> packDefaults = packDefaults();
Map<String, Object> rulesMap = new LinkedHashMap<>();
for (CustomLintRule desired : desiredRules) {
- String ruleId = desired.generateRuleId();
- CustomLintRule packDefault = packDefaults.get(ruleId);
- if (desired.getPackOwner() == RulePackOwner.PROJECT || packDefault ==
null) {
- rulesMap.put(ruleId, toFullCustomRuleMap(desired));
- } else if (structurallyDiffersFromPackDefault(desired, packDefault)) {
- // The project has redefined the rule rather than tuned it. Written
in full, it replaces
- // the pack rule of that id instead of layering on top of it.
- rulesMap.put(ruleId, toFullCustomRuleMap(desired));
- } else if (differsFromPackDefault(desired, packDefault)) {
- rulesMap.put(ruleId, toOverrideMap(desired, packDefault));
+ Map<String, Object> entry = entryFor(desired, packDefaults);
+ if (entry != null) {
+ rulesMap.put(desired.generateRuleId(), entry);
}
}
@@ -66,6 +56,43 @@ public final class ProjectLintYamlExporter {
}
}
+ /**
+ * What the project's {@code hop-lint.yml} has to say about one rule, so the
rule manager can
+ * write that rule alone and leave the rest of the file as the user wrote it.
+ *
+ * @return the keys to write under the rule's id, or null when the rule is
the pack's own and the
+ * project need say nothing about it
+ */
+ public static Map<String, Object> entryFor(CustomLintRule desired) {
+ return entryFor(desired, packDefaults());
+ }
+
+ private static Map<String, Object> entryFor(
+ CustomLintRule desired, Map<String, CustomLintRule> packDefaults) {
+ CustomLintRule packDefault = packDefaults.get(desired.generateRuleId());
+ if (desired.getPackOwner() == RulePackOwner.PROJECT || packDefault ==
null) {
+ return toFullCustomRuleMap(desired);
+ }
+ if (structurallyDiffersFromPackDefault(desired, packDefault)) {
+ // The project has redefined the rule rather than tuned it. Written in
full, it replaces the
+ // pack rule of that id instead of layering on top of it.
+ return toFullCustomRuleMap(desired);
+ }
+ if (differsFromPackDefault(desired, packDefault)) {
+ return toOverrideMap(desired, packDefault);
+ }
+ return null;
+ }
+
+ /** The rules as the packs define them, before any project changes them. */
+ private static Map<String, CustomLintRule> packDefaults() {
+ Map<String, CustomLintRule> packDefaults = new LinkedHashMap<>();
+ for (CustomLintRule rule :
RuleRegistry.getInstance().resolve(null).getRules()) {
+ packDefaults.put(rule.generateRuleId(), rule);
+ }
+ return packDefaults;
+ }
+
private static boolean differsFromPackDefault(
CustomLintRule desired, CustomLintRule packDefault) {
return desired.isEnabled() != packDefault.isEnabled()
@@ -165,7 +192,10 @@ public final class ProjectLintYamlExporter {
// Omitted when empty so an unrestricted rule round-trips to the same
YAML it came from.
ruleConfig.put("appliesTo", new ArrayList<>(rule.getAppliesTo()));
}
- ruleConfig.put("parameters", new
HashMap<>(rule.getAdditionalParameters()));
+ if (!rule.getAdditionalParameters().isEmpty()) {
+ // Omitted when empty, like appliesTo: "parameters: {}" on every rule
was noise in the diff.
+ ruleConfig.put("parameters", new
HashMap<>(rule.getAdditionalParameters()));
+ }
return ruleConfig;
}
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 4b38845164..a1b319030a 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
@@ -74,6 +74,7 @@ RuleBuilderDialog.Dialog.ValidationError.Title=Validation
Error
# Rule manager dialog
# ---------------------------------------------------------------------------
RuleManagerDialog.Title.EffectiveRules=Lint Rules ({0} effective rules)
+RuleManagerDialog.Column.RuleId=Rule ID
RuleManagerDialog.Column.RuleName=Rule Name
RuleManagerDialog.Column.Source=Source
RuleManagerDialog.Column.Target=Target
@@ -90,6 +91,8 @@ RuleManagerDialog.Button.Close=Close
RuleManagerDialog.Dialog.ConfirmDelete.Title=Confirm Delete
RuleManagerDialog.Dialog.ConfirmDelete.Message=Delete project rule ''{0}''?
RuleManagerDialog.Dialog.PackRule.Title=Pack Rule
+RuleManagerDialog.Dialog.SaveFailed.Title=Rule Not Saved
+RuleManagerDialog.Dialog.SaveFailed.Message=Rule {0} could not be saved to
hop-lint.yml. The file was not changed.
# ---------------------------------------------------------------------------
# Progress dialog
diff --git
a/plugins/misc/lint/src/test/java/org/apache/hop/lint/LintRuleYamlWriterTest.java
b/plugins/misc/lint/src/test/java/org/apache/hop/lint/LintRuleYamlWriterTest.java
new file mode 100644
index 0000000000..ca4fd65c91
--- /dev/null
+++
b/plugins/misc/lint/src/test/java/org/apache/hop/lint/LintRuleYamlWriterTest.java
@@ -0,0 +1,255 @@
+/*
+ * 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.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.hop.lint.registry.YamlRulePackParser;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+/**
+ * Saving one rule from the rule manager.
+ *
+ * <p>The rule manager used to write the whole of hop-lint.yml from its list
of rules. Toggling one
+ * rule deleted the exclude and suppress sections with their reasons, every
comment and the rule
+ * order, so excluded files were linted again and accepted findings came back
without a word.
+ *
+ * @see <a href="https://github.com/apache/hop/issues/8734";>#8734</a>
+ */
+public class LintRuleYamlWriterTest {
+
+ private static final String PROJECT_FILE =
+ """
+ # Lint policy for the CRM project, reviewed 2026-09-14
+ pack:
+ id: crm
+
+ rules:
+ # Too noisy on the generated pipelines
+ DOC-001:
+ enabled: false
+
+ SEC-002:
+ severity: WARNING # until the vault migration is done
+
+ CRM-001:
+ type: custom
+ enabled: true
+ severity: ERROR
+ target: PIPELINE
+ targetField: name
+ condition: MATCHES_PATTERN
+ conditionValue: '^crm_.*'
+ name: CRM pipeline names
+ description: Pipelines are named crm_<subject>
+
+ exclude:
+ # Metadata injection templates
+ - "templates/**"
+
+ suppress:
+ - rule: "SEC-002"
+ path: "load/http.hpl"
+ source: "HTTP client"
+ reason: "Test endpoint without credentials"
+ """;
+
+ @TempDir private Path dir;
+
+ private Path yaml() {
+ return dir.resolve("hop-lint.yml");
+ }
+
+ private static Map<String, Object> entry(Object... keysAndValues) {
+ Map<String, Object> entry = new LinkedHashMap<>();
+ for (int i = 0; i < keysAndValues.length; i += 2) {
+ entry.put((String) keysAndValues[i], keysAndValues[i + 1]);
+ }
+ return entry;
+ }
+
+ @Test
+ public void changingOneRuleLeavesEveryOtherLineAlone() throws Exception {
+ Files.writeString(yaml(), PROJECT_FILE, StandardCharsets.UTF_8);
+
+ LintPolicyYamlWriter.putRule(yaml(), "DOC-001", entry("enabled", true));
+
+ String after = Files.readString(yaml(), StandardCharsets.UTF_8);
+ assertEquals(
+ PROJECT_FILE.replace("DOC-001:\n enabled: false", "DOC-001:\n
enabled: true"), after);
+
+ LintPolicy policy =
YamlRulePackParser.parseProjectYaml(yaml().toFile()).getPolicy();
+ assertEquals(List.of("templates/**"), policy.getExcludes());
+ assertEquals(1, policy.getSuppressions().size());
+ }
+
+ @Test
+ public void aRuleWithAnInlineCommentIsReplacedWhole() throws Exception {
+ String after =
+ LintPolicyYamlWriter.replaceRule(PROJECT_FILE, "SEC-002",
entry("severity", "INFO"));
+
+ assertEquals(
+ PROJECT_FILE.replace(
+ "severity: WARNING # until the vault migration is done",
"severity: INFO"),
+ after);
+ }
+
+ @Test
+ public void theCommentAboveTheNextRuleStaysWithIt() throws Exception {
+ String file =
+ """
+ rules:
+ DOC-001:
+ enabled: false
+ # Too noisy on the generated pipelines
+ DOC-002:
+ enabled: false
+ """;
+
+ String after = LintPolicyYamlWriter.replaceRule(file, "DOC-001", null);
+
+ assertEquals(
+ """
+ rules:
+ # Too noisy on the generated pipelines
+ DOC-002:
+ enabled: false
+ """,
+ after);
+ }
+
+ @Test
+ public void aNewRuleIsAddedAtTheEndOfTheRules() throws Exception {
+ String after =
+ LintPolicyYamlWriter.replaceRule(PROJECT_FILE, "STRUCT-001",
entry("conditionValue", "40"));
+
+ assertTrue(
+ after.contains(" description: Pipelines are named crm_<subject>\n
STRUCT-001:\n"),
+ after);
+ assertTrue(after.contains(" STRUCT-001:\n conditionValue: '40'\n"),
after);
+ }
+
+ @Test
+ public void aComposedRuleIsWrittenAsBlockYaml() throws Exception {
+ Map<String, Object> composed =
+ entry(
+ "type",
+ "custom",
+ "target",
+ "TRANSFORM",
+ "allOf",
+ List.of(
+ entry("targetField", "sql", "condition",
"NOT_MATCHES_PATTERN"),
+ entry("targetField", "limit", "condition", "NOT_EMPTY")));
+
+ String after = LintPolicyYamlWriter.replaceRule(PROJECT_FILE, "SQL-002",
composed);
+
+ Map<?, ?> rules =
+ (Map<?, ?>) ((Map<?, ?>) new
org.yaml.snakeyaml.Yaml().load(after)).get("rules");
+ assertEquals(composed, rules.get("SQL-002"));
+ assertFalse(after.contains("{"), "flow style in a hand-edited file: " +
after);
+ }
+
+ @Test
+ public void theFileKeepsItsOwnIndentation() throws Exception {
+ String file =
+ """
+ rules:
+ DOC-001:
+ enabled: false
+ """;
+
+ String after = LintPolicyYamlWriter.replaceRule(file, "DOC-002",
entry("enabled", false));
+
+ assertTrue(after.contains("\n DOC-002:\n enabled: false"),
after);
+ }
+
+ @Test
+ public void aQuotedOrDifferentlyCasedKeyIsTheSameRule() throws Exception {
+ String file = """
+ rules:
+ 'doc-001':
+ enabled: false
+ """;
+
+ String after = LintPolicyYamlWriter.replaceRule(file, "DOC-001",
entry("enabled", true));
+
+ assertEquals("rules:\n DOC-001:\n enabled: true\n", after);
+ }
+
+ @Test
+ public void removingTheLastRuleRemovesTheRulesKey() throws Exception {
+ String file =
+ """
+ rules:
+ DOC-001:
+ enabled: false
+
+ exclude:
+ - "templates/**"
+ """;
+
+ String after = LintPolicyYamlWriter.replaceRule(file, "DOC-001", null);
+
+ assertEquals("\nexclude:\n - \"templates/**\"\n", after);
+ }
+
+ @Test
+ public void aFileWithoutRulesGetsTheBlockAppended() throws Exception {
+ String file = "exclude:\n - \"templates/**\"\n";
+
+ String after = LintPolicyYamlWriter.replaceRule(file, "DOC-001",
entry("enabled", false));
+
+ assertEquals(file + "\nrules:\n DOC-001:\n enabled: false\n", after);
+ }
+
+ @Test
+ public void anEmptyInlineRulesMappingIsOpenedUp() throws Exception {
+ String after =
+ LintPolicyYamlWriter.replaceRule("rules: {}\n", "DOC-001",
entry("enabled", false));
+
+ assertEquals("rules:\n DOC-001:\n enabled: false\n", after);
+ }
+
+ @Test
+ public void inlineRulesAreLeftToTheUser() {
+ assertThrows(
+ IOException.class,
+ () ->
+ LintPolicyYamlWriter.replaceRule(
+ "rules: {DOC-001: {enabled: false}}\n", "DOC-002",
entry("enabled", false)));
+ }
+
+ @Test
+ public void removingARuleThatIsNotThereChangesNothing() throws Exception {
+ Files.writeString(yaml(), PROJECT_FILE, StandardCharsets.UTF_8);
+
+ assertFalse(LintPolicyYamlWriter.removeRule(yaml(), "NAMING-001"));
+ assertEquals(PROJECT_FILE, Files.readString(yaml(),
StandardCharsets.UTF_8));
+ }
+}
diff --git
a/plugins/misc/lint/src/test/java/org/apache/hop/lint/RuleBuilderDialogNativeRuleTest.java
b/plugins/misc/lint/src/test/java/org/apache/hop/lint/RuleBuilderDialogNativeRuleTest.java
new file mode 100644
index 0000000000..d609256ea6
--- /dev/null
+++
b/plugins/misc/lint/src/test/java/org/apache/hop/lint/RuleBuilderDialogNativeRuleTest.java
@@ -0,0 +1,69 @@
+/*
+ * 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.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.concurrent.atomic.AtomicReference;
+import org.apache.hop.lint.registry.RuleRegistry;
+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;
+
+/**
+ * Editing a native rule, which says how Hop's own verify remarks are reported.
+ *
+ * <p>HOP-CHECK has no target, field or condition, so the editor opened it
with no target selected
+ * and refused every save with "Target type must be selected", even a change
of severity.
+ *
+ * @see <a href="https://github.com/apache/hop/issues/8734";>#8734</a>
+ */
+@Tag("uitest")
+class RuleBuilderDialogNativeRuleTest extends SwtBotTestBase {
+
+ @Test
+ void theSeverityOfHopCheckCanBeChanged() {
+ CustomLintRule hopCheck =
+ RuleRegistry.getInstance().resolve(null).getRules().stream()
+ .filter(rule -> "HOP-CHECK".equals(rule.generateRuleId()))
+ .findFirst()
+ .orElseThrow()
+ .copy();
+ AtomicReference<CustomLintRule> saved = new AtomicReference<>();
+ AtomicReference<Boolean> targetEnabled = new AtomicReference<>();
+
+ withDialog(
+ parent -> saved.set(new RuleBuilderDialog(parent, hopCheck).open()),
+ bot -> {
+ SWTBot dialog = bot.shell("Edit Lint Rule").bot();
+ targetEnabled.set(dialog.comboBoxWithLabel("Target
Type:").isEnabled());
+ dialog.comboBoxWithLabel("Severity:").setSelection("ERROR");
+ dialog.button("OK").click();
+ });
+
+ assertFalse(targetEnabled.get(), "a native rule has no target to choose");
+ assertNotNull(saved.get(), "the dialog refused to save");
+ assertEquals("ERROR", saved.get().getSeverity());
+ assertTrue(saved.get().isNativeVerify(), "still a native rule");
+ assertNull(saved.get().getTarget(), "and still without a target");
+ }
+}
diff --git
a/plugins/misc/lint/src/test/java/org/apache/hop/lint/RuleEditorChoicesTest.java
b/plugins/misc/lint/src/test/java/org/apache/hop/lint/RuleEditorChoicesTest.java
new file mode 100644
index 0000000000..bcb368798c
--- /dev/null
+++
b/plugins/misc/lint/src/test/java/org/apache/hop/lint/RuleEditorChoicesTest.java
@@ -0,0 +1,133 @@
+/*
+ * 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.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import org.apache.hop.lint.registry.HopCoreRulePack;
+import org.apache.hop.lint.registry.ProjectLintYamlExporter;
+import org.apache.hop.lint.registry.RuleRegistry;
+import org.junit.jupiter.api.Test;
+
+/**
+ * What the rule editor offers, and what the rule manager writes for one rule.
+ *
+ * <p>The editor guessed a field's type from its name and only offered the
matching conditions, so a
+ * rule on {@code isDummy}, {@code isReferenced} or {@code isOrphaned} opened
with an empty field or
+ * condition and could not be saved at all, not even to change its severity.
+ *
+ * @see <a href="https://github.com/apache/hop/issues/8734";>#8734</a>
+ */
+public class RuleEditorChoicesTest {
+
+ /** Every rule the core pack ships opens in the editor with its own field
and condition. */
+ @Test
+ public void everyCoreRuleFitsTheEditor() {
+ List<String> problems = new ArrayList<>();
+ for (CustomLintRule rule : new HopCoreRulePack().loadRules()) {
+ if (rule.isNativeVerify()) {
+ continue;
+ }
+ for (RuleClause clause : rule.getClauses()) {
+ String where = rule.generateRuleId() + " " + clause.getTargetField();
+ // A rule scoped to plugin types reads that plugin's own settings,
which no fixed list
+ // can name; the editor keeps such a field rather than offering it.
+ if (rule.getAppliesTo().isEmpty()
+ && !RuleTargetFields.getFieldsForTarget(rule.getTarget())
+ .contains(clause.getTargetField())) {
+ problems.add(where + ": field not offered for " + rule.getTarget());
+ }
+ if (!RuleTargetFields.getCompatibleConditions(clause.getTargetField())
+ .contains(clause.getCondition())) {
+ problems.add(where + ": " + clause.getCondition() + " not offered");
+ }
+ }
+ }
+ assertTrue(problems.isEmpty(), String.join("\n", problems));
+ }
+
+ @Test
+ public void isAndHasFieldsAreFlags() {
+ for (String field : List.of("isDummy", "isReferenced", "isOrphaned",
"hasNotes")) {
+ assertEquals(
+ List.of(RuleCondition.MUST_BE_TRUE, RuleCondition.MUST_BE_FALSE),
+ RuleTargetFields.getCompatibleConditions(field),
+ field);
+ }
+ for (String field : List.of("issuer", "hash", "isbn")) {
+ assertFalse(
+
RuleTargetFields.getCompatibleConditions(field).contains(RuleCondition.MUST_BE_TRUE),
+ field);
+ }
+ }
+
+ /** A Putki rule reads a Table Output's own truncateTable setting, and
PUTKI-ENV-006 a port. */
+ @Test
+ public void aFieldOrConditionTheEditorWouldNotSuggestIsKept() {
+ List<String> fields =
RuleTargetFields.getFieldChoices(RuleTarget.TRANSFORM, "truncateTable");
+ assertTrue(fields.contains("truncateTable"));
+
assertTrue(fields.containsAll(RuleTargetFields.getFieldsForTarget(RuleTarget.TRANSFORM)));
+
+ assertTrue(
+ RuleTargetFields.getConditionChoices("port",
RuleCondition.NO_HARDCODED)
+ .contains(RuleCondition.NO_HARDCODED));
+ assertEquals(
+ RuleTargetFields.getCompatibleConditions("port"),
+ RuleTargetFields.getConditionChoices("port", null));
+ }
+
+ @Test
+ public void anUnchangedPackRuleNeedsNoEntry() {
+ CustomLintRule rule = coreRule("DOC-001");
+
+ assertNull(ProjectLintYamlExporter.entryFor(rule));
+ }
+
+ @Test
+ public void aTunedPackRuleWritesOnlyWhatChanged() {
+ CustomLintRule rule = coreRule("DOC-001");
+ rule.setSeverity("INFO");
+
+ assertEquals(Map.of("severity", "INFO"),
ProjectLintYamlExporter.entryFor(rule));
+ }
+
+ @Test
+ public void aProjectRuleWithoutParametersWritesNone() {
+ CustomLintRule rule = coreRule("DOC-001");
+ rule.setId("CRM-001");
+ rule.setPackOwner(org.apache.hop.lint.registry.RulePackOwner.PROJECT);
+
+ Map<String, Object> entry = ProjectLintYamlExporter.entryFor(rule);
+
+ assertEquals("custom", entry.get("type"));
+ assertFalse(entry.containsKey("parameters"), entry.toString());
+ }
+
+ private static CustomLintRule coreRule(String id) {
+ return RuleRegistry.getInstance().resolve(null).getRules().stream()
+ .filter(rule -> id.equals(rule.generateRuleId()))
+ .findFirst()
+ .orElseThrow()
+ .copy();
+ }
+}
