zhangshenghang opened a new pull request, #12676:
URL: https://github.com/apache/seatunnel/pull/12676
## Purpose
While reviewing `seatunnel-ci-tools` I found several checks that either miss
real violations (false negatives) or can flag valid code (false positives).
This PR fixes the five high-impact ones. No new dependencies are introduced;
the symbol solver used by the new logic is the `javaparser-symbol-solver-core`
dependency the module already declares.
## Changes
**1. `ImportClassCheckTest.commonLang2Check` - false negative**
The lang2 check filtered whole *files* ("does this file also import
lang3?"), so a file mixing `org.apache.commons.lang.StringUtils` (lang2,
violation) with any lang3 import escaped detection. The check now excludes
lang3 per *import statement*.
**2. `SerialVersionUIDCheckerTest.setupTypeSolver` - performance + silent
no-op on Windows**
The source-root filter matched every path *containing* `src/main/java`, so a
`JavaParserTypeSolver` was registered for every nested directory and every
single `.java` file (tens of thousands of solvers that can never resolve
anything but are all tried on every symbol resolution). It also hardcoded `/`
as separator while the rest of the class adapts to the platform, so on Windows
no solver was registered at all, every `resolve()` failed and the whole check
silently passed. The filter now registers only real source-root directories,
matched with `Path#endsWith` which uses the platform separator.
**3. `SerialVersionUIDCheckerTest.isAbstractClass` - false positive risk**
Classes resolved outside the four checked paths are not in
`classDeclarationMap`, and the old fallback returned `false` (= "concrete"),
which could report abstract classes as missing `serialVersionUID`. The fallback
now inspects the AST of the resolved declaration and, when no AST is available
(e.g. types resolved from reflection), conservatively treats the class as
abstract and skips it.
**4. `SerialVersionUIDCheckerTest` / `ConnectorOptionCheckTest` - false
negative: only direct implements/extends were matched**
A connector implementing `SeaTunnelSource`/`SeaTunnelSink` through an
intermediate base class was only detected when that base happened to be on a
hardcoded list (which had to be extended repeatedly). Both checks now resolve
the type hierarchy transitively via the symbol solver and match the
fully-qualified ancestor names. The previous direct-name check is kept as
fallback for hierarchies that cannot be resolved from the source roots, so
behavior degrades gracefully instead of crashing. `checkImplementedTypes` now
also inspects the type arguments bound on *extended* types (`extends
IncrementalSource<...>`), not only implemented ones.
**5. `SerialVersionUIDCheckerTest` - the failure message promised a
validation that never ran**
The error message has always said "make sure its value is not -1L", but the
check only verified the field name existed. It now validates the declaration:
the field must be `static final long` and must not use the `-1L` sentinel. The
value check applies to the classes this checker directly governs (CDC / JDBC
config / engine job classes); transitively referenced state types are still
only checked for existence, so classes whose serialized form already ships with
`-1L` (e.g. `SeaTunnelRow`) do not silently change their wire format.
**6. Fix the violation the stricter check uncovered**
The transitive detection immediately found a real gap:
`OceanBaseIncrementalSource extends MySqlIncrementalSource ->
IncrementalSource` had no `OceanBaseIncrementalSourceOptions` class - every
other CDC connector has one. The new class inherits the MySQL CDC options and
carries the MySQL-compatible-mode constant, which the factory now references.
## Verification
- `./mvnw -B -pl seatunnel-ci-tools -am test` - all 34 tests pass (`BUILD
SUCCESS`)
- Negative test: a file importing both lang2 and lang3 is now flagged at the
exact import line (previously passed)
- Negative test: a `serialVersionUID = -1L` class in an engine job package
is now flagged with a clear message
- The transitive check was validated by the real
`OceanBaseIncrementalSourceOptions` finding above
## Does this PR introduce any user-facing change?
No.
--
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]
