bamaer opened a new issue, #8536:
URL: https://github.com/apache/hop/issues/8536
### Apache Hop version?
2.20.0-SNAPSHOT
### Java version?
21
### Operating system
Linux
### What happened?
## Summary
A project can raise, lower or silence *all* of Hop's own `check()` remarks,
or
none. It cannot say anything about one of them. Three separate things cause
this; they are grouped here because they have to be resolved together to make
"accept this one check" expressible.
Context: #8294 introduced `type: native` rules and the core pack's blanket
`HOP-CHECK` rule. #8531 made the classifier reach every path that reports a
native remark, so the CLI and the editor now agree. What they agree on is
still
"one rule for every check".
## 1. Every native remark reports under the same rule id
`NativeCheckClassifier.classify` returns the matching rule's id, and
`LintCheckResultAdapter.fromCheckResult` overwrites the remark's error code
with
it. The core pack's `HOP-CHECK` rule names no plugin and no message, so it
scores 0 and matches every remark put in front of it.
Rule ids are the suppression key: `LintSuppressGuiPlugin` writes the id the
person was shown into `hop-lint.yml`, and `LintPolicy.isSuppressed` matches
on
it. So a project can only accept every native remark on an element, or none.
Scope is small. Of 1454 `new CheckResult(...)` sites in main code, 7 pass an
error code: four in `ReferencedDatabaseConnectionChecker`, one
`CAN_START_WITHOUT_INPUT` in `PipelineMeta`, and two in the lint adapters
converting the other way. Everything else would keep falling back to
`HOP-CHECK`.
## 2. `messageKey` cannot name a check whose message takes parameters
`NativeCheckClassifier.printsMessage` resolves the key with no arguments and
asks whether the remark text contains the result. For a parameterised message
the resolved value still holds its placeholders, so it never matches:
resolved with no args : Database connection '{0}' assigned on {1} '{2}'
does not exist
actual remark text : Database connection 'NoSuchConnection' assigned
on Action 'Check DB connections' does not exist
printsMessage : false
A parameterless key matches correctly. A scan of the `check()` message keys
that
resolve to a bundle entry found roughly 240 parameterised against roughly
1140
in total, so the feature works for most checks and silently does nothing for
the
rest — including all four messages of the connection checker in point 1.
Failing closed is deliberate for a renamed key, and right for that case. It
is
wrong for a message that simply carries a `{0}`, and nothing reports it.
## 3. The editor reports the linter's own findings as Hop's
`CustomRuleExecutor.createResult` builds a policy finding with `Origin.LINT`
and
the rule's name. `HopLinter.lintPipelineLikeVerify`,
`HopLinter.lintWorkflowLikeVerify` and `PipelineVerifyLintExtension` then
round
trip it through `LintCheckResultAdapter.toCheckResults` and back;
`fromCheckResult` hard-codes `Origin.HOP_NATIVE`, replaces the rule name with
the source name, and leaves `[RULE-ID] RuleName: ` inside the message.
Two consequences:
- `LintResultDeduplicator.reduceGroup` collapses a native remark into a lint
finding only when the bucket holds an `Origin.LINT` finding. In the editor
none ever does, so the collapse never happens there. The command line
reports
one finding where the editor reports two.
- `LintResultsPanel` shows the rule id in its own column and again inside the
message, and its "Rule Name" detail shows a transform name.
## What has to be decided
Whether a specific error code survives as the rule id. It changes the id
those
findings report under, so an existing `hop-lint.yml` suppressing one of them
as
`HOP-CHECK` would stop matching. Either that is documented as a breaking
change,
or `LintPolicy.isSuppressed` accepts both the specific id and the id of the
rule
that classified the remark, and existing configuration keeps working.
Nothing else here needs a decision.
## Suggested order
1. Point 3. Independent of the rest, and the only one that changes how many
findings a user sees.
2. Point 2. Match the resolved pattern as a regex rather than a literal, and
warn at rule-load time when a `messageKey` resolves to nothing.
3. Point 1. Smallest diff, but carries the decision above.
`NativeCheckClassifierTest` and `LintSuppressionInEditorTest` are the right
homes for the tests.
### Issue Priority
Priority: 3
### Issue Component
Component: Hop Gui
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
