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 965c847b39 Fixes #8733 : Recognise Hop Gui's default names, fix
SQL-002, add IS_… (#8743)
965c847b39 is described below
commit 965c847b397fa0f194bae1398b04a1d9cebedc37
Author: Bart Maertens <[email protected]>
AuthorDate: Thu Oct 8 19:32:57 2026 +0200
Fixes #8733 : Recognise Hop Gui's default names, fix SQL-002, add IS_…
(#8743)
* Fixes #8733 : Recognise Hop Gui's default names, fix SQL-002, add
IS_EMPTY and read composed rules without type: custom
* Issue #8733 : Report a missing or negative row limit in SQL-002, describe
the names NAMING-004 recognises, correct the IS_EMPTY example
---
.../modules/ROOT/pages/linting/lint-rules.adoc | 20 +-
.../org/apache/hop/lint/CustomRuleExecutor.java | 96 +++++++++-
.../main/java/org/apache/hop/lint/LintCommand.java | 12 ++
.../java/org/apache/hop/lint/RuleCondition.java | 1 +
.../org/apache/hop/lint/RuleManagerDialog.java | 4 +-
.../java/org/apache/hop/lint/RuleTargetFields.java | 1 +
.../hop/lint/registry/YamlRulePackParser.java | 15 +-
.../misc/lint/src/main/resources/hop-lint-core.yml | 14 +-
.../hop/lint/messages/messages_en_US.properties | 6 +-
.../org/apache/hop/lint/CoreRulePackFixesTest.java | 207 +++++++++++++++++++++
10 files changed, 355 insertions(+), 21 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 7486c4fc12..94ce4de96d 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
@@ -94,6 +94,9 @@ Scope such a rule with `appliesTo`, naming the metadata key —
the folder name
|`NOT_EMPTY`, `NOT_NULL`
|has a value
+|`IS_EMPTY`
+|has no value
+
|`NOT_EMPTY_COLLECTION`
|is a collection with at least one entry
@@ -140,7 +143,7 @@ Scope such a rule with `appliesTo`, naming the metadata key
— the folder name
|Whether any other file in the project calls this pipeline or workflow. Only a
project lint can answer this, see <<project-rules>>
|`hasDefaultName`
-|The transform or action still carries its auto-generated name
+|The transform or action still carries the name Hop Gui gave it: the plugin's
name, such as `Table input`, followed by a number when that name was taken, as
in `Dummy (do nothing) 2`. The plugin name is the one of the language Hop runs
in. A workflow's Start action never counts
|`isDummy`
|The transform is a Dummy
@@ -183,15 +186,22 @@ Some problems are only worth reporting when several
things are wrong together, a
- TableInput
allOf:
- targetField: sql
- condition: MATCHES_PATTERN
+ condition: NOT_MATCHES_PATTERN
conditionValue: "(?is).*\\bselect\\s+\\*.*"
- targetField: rowLimit
- condition: NOT_EMPTY
+ condition: NOT_MATCHES_PATTERN
+ conditionValue: "^\\s*([+-]?0*|-\\d+)\\s*$"
name: "Unbounded SELECT *"
description: "A Table Input that selects every column and sets no row
limit"
----
-`allOf` reports only when *every* clause is broken, `anyOf` when *at least
one* is.
+As everywhere else, each condition says what is *required*, and the rule
reports when that is not met.
+`allOf` reports only when *every* condition is not met, `anyOf` when *at least
one* is not.
+The rule above reports a Table Input whose SQL selects every column *and*
whose row limit is unset, `0` or negative, which Hop reads as no limit.
+
+To report something that is set, require it to be empty: a plain `http://` URL
with a login is `url` `MATCHES_PATTERN` `^https://.*` (not met for `http://`)
together with `httpLogin` `IS_EMPTY` (not met when a login is set), under
`allOf`.
+
+A rule with `allOf` or `anyOf` and a `target` is a rule definition even
without `type: custom`.
Each clause names its own field, condition and value, and the finding says
which clauses were broken and what the values actually were.
A rule with neither block behaves exactly as before, so nothing that was
written against the single-condition form needs changing.
@@ -298,7 +308,7 @@ Hop's core rule pack is deliberately small. A rule is
enabled by default only wh
|`NAMING-004`
|WARNING
-|A transform still carrying its auto-generated name
+|A transform still carrying the name Hop Gui gave it, such as `Table input` or
`Table input 2`
|`HOP-CHECK`
|WARNING
diff --git
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/CustomRuleExecutor.java
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/CustomRuleExecutor.java
index faeec3e56e..65b133bb7e 100644
---
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/CustomRuleExecutor.java
+++
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/CustomRuleExecutor.java
@@ -20,11 +20,19 @@ import java.lang.reflect.Field;
import java.lang.reflect.Modifier;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Collections;
+import java.util.LinkedHashMap;
import java.util.List;
+import java.util.Map;
import java.util.regex.Pattern;
import org.apache.hop.core.database.DatabaseMeta;
import org.apache.hop.core.logging.ILogChannel;
import org.apache.hop.core.logging.LogChannel;
+import org.apache.hop.core.plugins.ActionPluginType;
+import org.apache.hop.core.plugins.IPlugin;
+import org.apache.hop.core.plugins.IPluginType;
+import org.apache.hop.core.plugins.PluginRegistry;
+import org.apache.hop.core.plugins.TransformPluginType;
import org.apache.hop.core.util.Utils;
import org.apache.hop.metadata.api.HopMetadata;
import org.apache.hop.metadata.api.HopMetadataProperty;
@@ -483,6 +491,41 @@ public class CustomRuleExecutor {
return null;
}
+ /**
+ * The fields every transform has, whatever its plugin, with their types.
The linter works them
+ * out itself rather than reading them from the plugin, so {@code hop lint
--list-fields} lists
+ * them from here. Keep in step with {@link #extractFieldFromTransform}.
+ */
+ public static final Map<String, String> TRANSFORM_FIELDS =
+ orderedFields(
+ "name", "String",
+ "description", "String",
+ "pluginId", "String",
+ "copies", "int",
+ "isDummy", "boolean",
+ "hasDefaultName", "boolean",
+ "isOrphaned", "boolean",
+ "isBlockingTransform", "boolean");
+
+ /**
+ * As {@link #TRANSFORM_FIELDS}, for actions. Keep in step with {@link
#extractFieldFromAction}.
+ */
+ public static final Map<String, String> ACTION_FIELDS =
+ orderedFields(
+ "name", "String",
+ "description", "String",
+ "pluginId", "String",
+ "hasDefaultName", "boolean",
+ "isOrphaned", "boolean");
+
+ private static Map<String, String> orderedFields(String... namesAndTypes) {
+ Map<String, String> fields = new LinkedHashMap<>();
+ for (int i = 0; i < namesAndTypes.length; i += 2) {
+ fields.put(namesAndTypes[i], namesAndTypes[i + 1]);
+ }
+ return Collections.unmodifiableMap(fields);
+ }
+
/** Extract field value from a transform using reflection */
private static Object extractFieldFromTransform(
TransformMeta transformMeta, String fieldName, CustomLintRule rule) {
@@ -501,7 +544,10 @@ public class CustomRuleExecutor {
case "isDummy":
return
"Dummy".equalsIgnoreCase(transformMeta.getTransformPluginId());
case "hasDefaultName":
- return hasDefaultGeneratedName(transformMeta.getName());
+ return hasDefaultGeneratedName(
+ transformMeta.getName(),
+ transformMeta.getTransformPluginId(),
+ TransformPluginType.class);
case "isOrphaned":
return SUBJECT.get() instanceof PipelineMeta pipeline
? isOrphaned(transformMeta, pipeline.getPipelineHops(),
pipeline.getTransforms())
@@ -534,7 +580,12 @@ public class CustomRuleExecutor {
case "pluginId":
return actionMeta.getAction().getPluginId();
case "hasDefaultName":
- return hasDefaultGeneratedName(actionMeta.getName());
+ // Every workflow starts at Start; there is no better name for it.
+ return !actionMeta.isStart()
+ && hasDefaultGeneratedName(
+ actionMeta.getName(),
+ actionMeta.getAction().getPluginId(),
+ ActionPluginType.class);
case "isOrphaned":
return SUBJECT.get() instanceof WorkflowMeta workflow
? isOrphaned(actionMeta, workflow.getWorkflowHops(),
workflow.getActions())
@@ -553,11 +604,35 @@ public class CustomRuleExecutor {
}
}
- private static boolean hasDefaultGeneratedName(String name) {
+ /**
+ * Whether a transform or action still has the name Hop Gui gave it: the
plugin's name, such as
+ * "Table input", followed by a number when that name was taken, as in
"Dummy (do nothing) 2".
+ *
+ * <p>Only "Transform 1" and "Action 1" used to count, and Hop Gui never
generates those. The
+ * plugin's name is the one of the language Hop runs in, so a name Hop Gui
gave in another
+ * language is not recognised.
+ */
+ static boolean hasDefaultGeneratedName(
+ String name, String pluginId, Class<? extends IPluginType> pluginType) {
if (Utils.isEmpty(name)) {
return false;
}
- return name.matches("(?i)(Transform|Action)\\s+\\d+");
+ String trimmed = name.trim();
+ if (trimmed.matches("(?i)(Transform|Action)\\s+\\d+")) {
+ return true;
+ }
+ if (Utils.isEmpty(pluginId)) {
+ return false;
+ }
+ IPlugin plugin = PluginRegistry.getInstance().findPluginWithId(pluginType,
pluginId);
+ if (plugin == null || Utils.isEmpty(plugin.getName())) {
+ return false;
+ }
+ String pluginName = plugin.getName().trim();
+ return trimmed.equalsIgnoreCase(pluginName)
+ || (trimmed.length() > pluginName.length() + 1
+ && trimmed.substring(0,
pluginName.length()).equalsIgnoreCase(pluginName)
+ && trimmed.substring(pluginName.length()).matches("\\s+\\d+"));
}
/**
@@ -1000,7 +1075,12 @@ public class CustomRuleExecutor {
// fire on it. Hop returns null for an unset description, which is
exactly the case
// "description NOT_EMPTY" exists to catch; treating null as passing
made those rules
// fire only on a description explicitly set to "".
- return condition == RuleCondition.NOT_NULL || condition ==
RuleCondition.NOT_EMPTY;
+ if (condition != RuleCondition.NOT_MATCHES_PATTERN) {
+ return condition == RuleCondition.NOT_NULL || condition ==
RuleCondition.NOT_EMPTY;
+ }
+ // A pattern that forbids blank values, such as SQL-002's "no row
limit", must also see an
+ // unset value, so NOT_MATCHES_PATTERN reads null as "".
+ fieldValue = "";
}
// Handle null condition value for conditions that don't need it
@@ -1024,6 +1104,12 @@ public class CustomRuleExecutor {
case NOT_EMPTY:
return Utils.isEmpty(fieldValue.toString());
+ case IS_EMPTY:
+ // The opposite of NOT_EMPTY, so that a clause can require that a
field is not set. Under
+ // allOf, url MATCHES_PATTERN ^https://.* and httpLogin IS_EMPTY
report a plain http:// URL
+ // with a login.
+ return !Utils.isEmpty(fieldValue.toString());
+
case NOT_NULL:
return fieldValue == null;
diff --git
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintCommand.java
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintCommand.java
index f481f76ae8..223e6320a1 100644
--- a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintCommand.java
+++ b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintCommand.java
@@ -532,6 +532,18 @@ public class LintCommand implements Callable<Integer>,
IHopCommand, IHasHopMetad
System.out.printf(" %-40s %s%n", entry.getKey(), entry.getValue());
}
System.out.println();
+ // hasDefaultName and the like are worked out by the linter, not read from
the plugin, and
+ // were missing from this list although a rule can use them on any
transform or action.
+ Map<String, String> common =
+ "transform".equals(kind)
+ ? CustomRuleExecutor.TRANSFORM_FIELDS
+ : CustomRuleExecutor.ACTION_FIELDS;
+ System.out.println("Fields on every " + kind + ":");
+ System.out.println();
+ for (Map.Entry<String, String> entry : common.entrySet()) {
+ System.out.printf(" %-40s %s%n", entry.getKey(), entry.getValue());
+ }
+ System.out.println();
System.out.println(
"Nested values are reached with a dotted path, for example
fileSettings.fileName.");
return 0;
diff --git
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/RuleCondition.java
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/RuleCondition.java
index 36e7a29083..5ac953f67e 100644
--- a/plugins/misc/lint/src/main/java/org/apache/hop/lint/RuleCondition.java
+++ b/plugins/misc/lint/src/main/java/org/apache/hop/lint/RuleCondition.java
@@ -26,6 +26,7 @@ public enum RuleCondition {
// String conditions
NOT_EMPTY("Not Empty", "Field must not be empty or null", false),
NOT_NULL("Not Null", "Field must not be null", false),
+ IS_EMPTY("Is Empty", "Field must be empty or not set", false),
NO_HARDCODED("No Hardcoded Values", "Field must use variables, not hardcoded
values", false),
MATCHES_PATTERN("Matches Pattern", "Field must match specified regex
pattern", true),
NOT_MATCHES_PATTERN(
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 101b8a3b59..a9f673b649 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
@@ -294,7 +294,9 @@ public class RuleManagerDialog extends Dialog {
if (rule.isComposed()) {
item.setText(
4, rule.getClauses().size() + " fields (" +
rule.getCombinator().getYamlKey() + ")");
- item.setText(5, rule.getCombinator() == RuleCombinator.ALL_OF ? "All
of" : "Any of");
+ // A condition says what is required; the rule reports when the
conditions are not met.
+ item.setText(
+ 5, rule.getCombinator() == RuleCombinator.ALL_OF ? "All not met" :
"Any not met");
item.setText(6, "");
} else {
item.setText(5, rule.getCondition() != null ?
rule.getCondition().getDisplayName() : "");
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 4c7185f444..c0faa829d8 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
@@ -225,6 +225,7 @@ public class RuleTargetFields {
return Arrays.asList(
RuleCondition.NOT_EMPTY,
RuleCondition.NOT_NULL,
+ RuleCondition.IS_EMPTY,
RuleCondition.NO_HARDCODED,
RuleCondition.MATCHES_PATTERN,
RuleCondition.NOT_MATCHES_PATTERN,
diff --git
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/registry/YamlRulePackParser.java
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/registry/YamlRulePackParser.java
index 9e00b9e16b..d809b28e1d 100644
---
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/registry/YamlRulePackParser.java
+++
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/registry/YamlRulePackParser.java
@@ -399,7 +399,20 @@ public final class YamlRulePackParser {
if ("custom".equals(type)) {
return true;
}
- return ruleData.containsKey("target") && ruleData.containsKey("condition");
+ if (!ruleData.containsKey("target")) {
+ return false;
+ }
+ // A composed rule has its conditions inside allOf or anyOf rather than at
the top. Without
+ // type: custom it was taken for an override of a rule that does not
exist, and dropped.
+ if (ruleData.containsKey("condition")) {
+ return true;
+ }
+ for (RuleCombinator combinator : RuleCombinator.values()) {
+ if (ruleData.containsKey(combinator.getYamlKey())) {
+ return true;
+ }
+ }
+ return false;
}
public static boolean isNativeRuleDefinition(Map<String, Object> ruleData) {
diff --git a/plugins/misc/lint/src/main/resources/hop-lint-core.yml
b/plugins/misc/lint/src/main/resources/hop-lint-core.yml
index 423f93a716..629626f6e6 100644
--- a/plugins/misc/lint/src/main/resources/hop-lint-core.yml
+++ b/plugins/misc/lint/src/main/resources/hop-lint-core.yml
@@ -137,7 +137,7 @@ rules:
targetField: hasDefaultName
condition: MUST_BE_FALSE
name: "Default Transform Name"
- description: "Auto-generated names such as 'Transform 1' make logs and
error messages hard to trace back to the pipeline"
+ description: "Names Hop Gui assigns, such as 'Table input' or 'Table input
2', make logs and error messages hard to trace back to the pipeline"
# ------------------------------------------------------------------
# Hop's own verify remarks.
@@ -303,9 +303,10 @@ rules:
name: "Workflow Naming Convention"
description: "Example naming rule: UPPER_SNAKE_CASE workflow names.
Replace conditionValue with your own pattern"
- # A worked example of a rule that checks more than one thing. allOf reports
only when every
- # clause is broken, so a SELECT * that is bounded by a row limit is left
alone; anyOf would
- # report when either was. Each clause names its own field, condition and
value.
+ # A worked example of a rule that checks more than one thing. Each condition
says what is
+ # required, and allOf reports only when every one of them is not met: here,
when the SQL selects
+ # every column and the row limit is unset, 0 or negative, which Hop reads as
no limit. anyOf would report
+ # when either was not met. Each clause names its own field, condition and
value.
SQL-002:
type: custom
enabled: false
@@ -315,10 +316,11 @@ rules:
- TableInput
allOf:
- targetField: sql
- condition: MATCHES_PATTERN
+ condition: NOT_MATCHES_PATTERN
conditionValue: "(?is).*\\bselect\\s+\\*.*"
- targetField: rowLimit
- condition: NOT_EMPTY
+ condition: NOT_MATCHES_PATTERN
+ conditionValue: "^\\s*([+-]?0*|-\\d+)\\s*$"
name: "Unbounded SELECT *"
description: "A Table Input that selects every column and sets no row
limit reads more than it needs, and breaks when the table gains a column"
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 a1b319030a..94a2ec2775 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
@@ -122,9 +122,9 @@ LintResultsPanel.Details.GroupSelected=Group selected.
Select a specific issue t
# ---------------------------------------------------------------------------
PreCommitLintExtension.Dialog.CommitBlocked.Title=Commit Blocked - Lint {0}+
Issues
PreCommitLintExtension.CommitBlocked.Reason={0} lint issue(s) at {1} or above.
Open Tools > Lint > Show Lint Results for the full list.
-RuleBuilderDialog.Label.Match=Match:
-RuleBuilderDialog.Combinator.AllOf=All of the clauses below
-RuleBuilderDialog.Combinator.AnyOf=Any of the clauses below
+RuleBuilderDialog.Label.Match=Report when:
+RuleBuilderDialog.Combinator.AllOf=every condition below is not met
+RuleBuilderDialog.Combinator.AnyOf=any condition below is not met
RuleBuilderDialog.Button.AddClause=Add clause
RuleBuilderDialog.Button.RemoveClause=Remove clause
LintResultsPanel.Shell.TitleForFolder=Hop Lint Results \u2014 folder {0}
diff --git
a/plugins/misc/lint/src/test/java/org/apache/hop/lint/CoreRulePackFixesTest.java
b/plugins/misc/lint/src/test/java/org/apache/hop/lint/CoreRulePackFixesTest.java
new file mode 100644
index 0000000000..f74a5674a4
--- /dev/null
+++
b/plugins/misc/lint/src/test/java/org/apache/hop/lint/CoreRulePackFixesTest.java
@@ -0,0 +1,207 @@
+/*
+ * 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.assertTrue;
+
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.PrintStream;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.util.List;
+import org.apache.hop.core.HopEnvironment;
+import org.apache.hop.core.plugins.TransformPluginType;
+import org.apache.hop.lint.registry.RuleRegistry;
+import org.apache.hop.pipeline.PipelineMeta;
+import org.apache.hop.pipeline.transform.BaseTransformMeta;
+import org.apache.hop.pipeline.transform.TransformMeta;
+import org.apache.hop.workflow.action.ActionMeta;
+import org.apache.hop.workflow.actions.start.ActionStart;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import picocli.CommandLine;
+
+/**
+ * Fixes to the core rule pack and to how rules are read and evaluated.
+ *
+ * @see <a href="https://github.com/apache/hop/issues/8733";>#8733</a>
+ */
+public class CoreRulePackFixesTest {
+
+ @BeforeAll
+ static void loadPlugins() throws Exception {
+ HopEnvironment.init();
+ }
+
+ private static CustomLintRule coreRule(String id) {
+ CustomLintRule rule =
+ RuleRegistry.getInstance().resolve(null).getRules().stream()
+ .filter(r -> id.equals(r.generateRuleId()))
+ .findFirst()
+ .orElseThrow()
+ .copy();
+ rule.setEnabled(true);
+ return rule;
+ }
+
+ // ------------------------------------------------------------------
NAMING-004
+
+ /**
+ * Hop Gui names a new transform after its plugin, with a number when the
name is taken. Only
+ * "Transform 1" counted, which Hop Gui never generates.
+ */
+ @Test
+ public void theNamesHopGuiGeneratesAreDefaultNames() {
+ for (String name :
+ List.of(
+ "Dummy (do nothing)", "Dummy (do nothing) 2", "dummy (DO nothing)
12", "Transform 1")) {
+ assertTrue(
+ CustomRuleExecutor.hasDefaultGeneratedName(name, "Dummy",
TransformPluginType.class),
+ name);
+ }
+ for (String name : List.of("Load customers", "Dummy (do nothing) copy",
"Dummy")) {
+ assertFalse(
+ CustomRuleExecutor.hasDefaultGeneratedName(name, "Dummy",
TransformPluginType.class),
+ name);
+ }
+ }
+
+ @Test
+ public void naming004ReportsADefaultTransformName() {
+ TransformMeta dummy = new TransformMeta();
+ dummy.setName("Dummy (do nothing) 2");
+ dummy.setTransformPluginId("Dummy");
+
+ assertEquals(
+ 1, CustomRuleExecutor.executeRule(coreRule("NAMING-004"), dummy,
"/tmp/p.hpl").size());
+ }
+
+ /** Every workflow starts at Start, and there is no better name for it. */
+ @Test
+ public void theStartActionNeverHasADefaultName() {
+ ActionMeta start = new ActionMeta(new ActionStart());
+ start.setName("Start");
+ CustomLintRule rule = coreRule("NAMING-004");
+ rule.setTarget(RuleTarget.ACTION);
+
+ assertTrue(CustomRuleExecutor.executeRule(rule, start,
"/tmp/w.hwf").isEmpty());
+ }
+
+ @Test
+ public void listFieldsShowsTheFieldsEveryTransformHas() {
+ ByteArrayOutputStream out = new ByteArrayOutputStream();
+ PrintStream systemOut = System.out;
+ try {
+ System.setOut(new PrintStream(out, true, StandardCharsets.UTF_8));
+ new CommandLine(new LintCommand()).execute("--list-fields", "Dummy");
+ } finally {
+ System.setOut(systemOut);
+ }
+
+ String listing = out.toString(StandardCharsets.UTF_8);
+ assertTrue(listing.contains("Fields on every transform:"), listing);
+ assertTrue(listing.contains("hasDefaultName"), listing);
+ assertTrue(listing.contains("isOrphaned"), listing);
+ }
+
+ // ------------------------------------------------------------------
SQL-002 and allOf
+
+ /** Stands in for a Table Input: the two fields SQL-002 reads. */
+ public static class FakeTableInputMeta extends BaseTransformMeta {
+ private String sql;
+ private String rowLimit;
+
+ FakeTableInputMeta(String sql, String rowLimit) {
+ this.sql = sql;
+ this.rowLimit = rowLimit;
+ }
+ }
+
+ private static boolean sql002Reports(String sql, String rowLimit) {
+ TransformMeta tableInput =
+ new TransformMeta("TableInput", "Read customers", new
FakeTableInputMeta(sql, rowLimit));
+ return !CustomRuleExecutor.executeRule(coreRule("SQL-002"), tableInput,
"/tmp/p.hpl").isEmpty();
+ }
+
+ /** Both clauses were the wrong way round, so the rule could not fire on
what it describes. */
+ @Test
+ public void sql002ReportsOnlyAnUnboundedSelectStar() {
+ assertTrue(sql002Reports("SELECT * FROM customers", "0"), "SELECT *, no
limit");
+ assertTrue(sql002Reports("select *\nfrom customers", ""), "SELECT *, empty
limit");
+ assertTrue(sql002Reports("SELECT * FROM customers", null), "SELECT *,
limit never set");
+ assertTrue(sql002Reports("SELECT * FROM customers", "-1"), "SELECT *,
negative limit");
+ assertTrue(sql002Reports("SELECT * FROM customers", "+0"), "SELECT *,
limit +0");
+ assertFalse(sql002Reports("SELECT * FROM customers", "100"), "SELECT *,
limit 100");
+ assertFalse(sql002Reports("SELECT * FROM customers", "${LIMIT}"), "limit
from a variable");
+ assertFalse(sql002Reports("SELECT id, name FROM customers", "0"), "named
columns");
+ }
+
+ @Test
+ public void isEmptyRequiresTheFieldToBeUnset() {
+ CustomLintRule rule = new CustomLintRule();
+ rule.setEnabled(true);
+ rule.setSeverity("WARNING");
+ rule.setTarget(RuleTarget.PIPELINE);
+ rule.setTargetField("description");
+ rule.setCondition(RuleCondition.IS_EMPTY);
+
+ PipelineMeta pipeline = new PipelineMeta();
+ pipeline.setName("load");
+ assertTrue(CustomRuleExecutor.executeRule(rule, pipeline,
"/tmp/p.hpl").isEmpty(), "unset");
+
+ pipeline.setDescription("Loads the customers");
+ assertEquals(1, CustomRuleExecutor.executeRule(rule, pipeline,
"/tmp/p.hpl").size(), "set");
+ }
+
+ /**
+ * An allOf rule written without type: custom was taken for an override of a
rule that does not
+ * exist, and never ran.
+ */
+ @Test
+ public void aComposedRuleNeedsNoTypeCustom() throws Exception {
+ File projectYaml = File.createTempFile("hop-lint", ".yml");
+ Files.writeString(
+ projectYaml.toPath(),
+ """
+ rules:
+ HTTP-001:
+ target: TRANSFORM
+ severity: WARNING
+ allOf:
+ - targetField: url
+ condition: MATCHES_PATTERN
+ conditionValue: "^https://.*";
+ - targetField: httpLogin
+ condition: IS_EMPTY
+ """);
+ try {
+ CustomLintRule rule =
+ RuleRegistry.getInstance().resolve(projectYaml).getRules().stream()
+ .filter(r -> "HTTP-001".equals(r.generateRuleId()))
+ .findFirst()
+ .orElseThrow();
+
+ assertEquals(2, rule.getClauses().size());
+ assertEquals(RuleCombinator.ALL_OF, rule.getCombinator());
+ } finally {
+ Files.deleteIfExists(projectYaml.toPath());
+ }
+ }
+}
