openrewrite / openrewrite/rewrite
Most language `Assertions` ignore `TypeValidation` entirely, including `unknown()`
Nobody has claimed this yet.
- Dominant language
- Java
- Stars
- 3.7k
- Forks
- 570
- Avg merge
- 13h 12m
- Merged PRs (30d)
- 261
Description
Background
TypeValidation reaches a test only through SourceSpec.validateSource, which RewriteTest invokes for the before-source and for the after-recipe result (rewrite-test/src/main/java/org/openrewrite/test/RewriteTest.java:350 and :498). Each language's Assertions chooses what to pass, and most pass SourceSpec.ValidateSource.noop — either explicitly, or implicitly via the 5-arg SourceSpec constructor that defaults to it (rewrite-test/src/main/java/org/openrewrite/test/SourceSpec.java:79).
The result is that in those modules no TypeValidation invariant is checked at all — not unknown(), not erroneous(), not the type-metadata flags — even though every one of their compilation units implements JavaSourceFile, which is the only thing org.openrewrite.java.Assertions.validateTypes requires.
| Module | validateSource |
unknown() honored |
|---|---|---|
| rewrite-java | Assertions::validateTypes |
yes |
-
| rewrite-kotlin |
Assertions::validateTypes| yes, since #8557 |
| rewrite-groovy, rewrite-gradle, rewrite-scala, rewrite-python, rewrite-javascript, rewrite-csharp, rewrite-ruby, rewrite-go |ValidateSource.noop| no | -
This is not theoretical.
ScalaParserVisitorwraps a whole file in aJ.Unknownwhen no statement converts (rewrite-scala/src/main/java/org/openrewrite/scala/ScalaParserVisitor.java:103-120), the TypeScript parser falls back tovisitUnknownin a dozen places, and C#/Ruby/Python all carryvisitUnknownin their visitors. #8557 is the cautionary tale: aJ.Unknownswallowing an entire top-level Kotlin declaration passed CI unnoticed until the check was added.
Corroborating signal: typeValidationOptions(...) appears in zero tests across groovy, scala, python, javascript, csharp, ruby and go. Nothing has ever needed to opt out, because nothing is on.
Out of scope: rewrite-maven and rewrite-docker
Both were checked and deliberately excluded — their trees cannot hold what the remaining flags check:
- rewrite-maven only builds
SourceSpec<Xml.Document>, andXml.Document implements Xml, SourceFileWithReferences— noJin the tree, sounknown()/erroneous()/type-metadata are unreachable.dependencyModel()is the only meaningful flag andpomResolvedSuccessfullyalready implements it. - rewrite-docker only builds
SourceSpec<Docker.File>, andDocker extends Treewith noorg.openrewrite.javadependency at all. ItsvalidatehandlesallowNonWhitespaceInWhitespace()and must stay: rewrite-docker ships noWhitespaceValidationService, so the generic check inRewriteTest.java:383throwsUnsupportedOperationExceptionand is silently skipped, and the Docker visitor additionally strips line continuations.
Everything else (parseAndPrintEquality, cursorAcyclic, immutableExecutionContext, immutableScanning, and allowNonWhitespaceInWhitespace where a WhitespaceValidationService exists) is enforced by RewriteTest generically, independent of validateSource.
A. Honor unknown() / erroneous() per language
- Each of these replaces
ValidateSource.noopwith a validator that fails onJ.UnknownwhentypeValidation.unknown()and onJ.ErroneouswhentypeValidation.erroneous(), modeled onorg.openrewrite.kotlin.Assertions#assertNoUnknownElements(see #8557). Independent of each other; do B1 first if you want to avoid copy-paste.
Expect existing tests in the module to start failing — that is the point. Each failure is either a parser gap worth fixing or an explicit .typeValidationOptions(TypeValidation.builder().unknown(false).build()) with a linked issue. Do not enable the type-metadata flags as part of these (see C1).
- A1 — rewrite-groovy (
groovy(...)) - A2 — rewrite-gradle:
buildGradle/settingsGradle(G.CompilationUnit) andbuildGradleKts/settingsGradleKts(K.CompilationUnit). The kts specs build a Kotlin LST but bypass the check added in #8557 - A3 — rewrite-scala (
scala(...),sbt(...)) - A4 — rewrite-python (
python(...)) - A5 — rewrite-javascript, Java-side
Assertions(javascript/typescript/jsx/tsx) - A6 — rewrite-csharp (
csharp(...)) - A7 — rewrite-ruby (
ruby(...),gemfile(...),rakefile(...)) - A8 — rewrite-go (
go(...)) https://github.com/openrewrite/rewrite/pull/8573
B. Shared plumbing and small fixes
- B1 — Extract the
J.Unknown/J.Erroneousscan into one reusable helper (rewrite-java and rewrite-kotlin already carry near-identical copies) so A1–A8 delegate instead of duplicating ~20 lines each. The Kotlin variant is the better base: it surfaces theParseExceptionResultmessage alongside the offending source text - B2 —
org.openrewrite.java.Assertions#validateTypesreports unknowns with the message"LST contains erroneous nodes", identical to theJ.Erroneousbranch directly above it, and drops theParseExceptionResultmessage. Fix the wording and include the parse exception - B3 — The TypeScript-side harness (
rewrite-javascript/rewrite/src/test/rewrite-test.ts) has no unknown/erroneous validation either, so TS-authored parser tests are blind to it in the same way. Add the equivalent there - B4 —
TypeValidation.none()is documented as "Skip all invariant validation checks" but passesallowNonWhitespaceInWhitespace = false, which enables whitespace validation. That flag's polarity is inverted relative to its siblings; decide whethernone()should passtrueor whether the field should be renamed
C. Needs a decision before implementing
- C1 — Per language, decide whether the type-metadata flags (
identifiers,methodInvocations,methodDeclarations,classDeclarations,constructorInvocations,allowMissingType) can ever be enabled. For the dynamic languages this likely stays off permanently; if so, say so in theAssertionsjavadoc so the next reader does not assume it is an oversight - C2 —
dependencyModel()is implemented only by rewrite-maven. Decide whether Gradle specs should assert theGradleProject/GradleSettingsmarker the waypomXmlassertsMavenResolutionResult
Quick way to see the current state: grep -rn "ValidateSource.noop" --include=Assertions.java ., plus every Assertions that uses the 5-arg SourceSpec constructor.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the ValidateSource.noop usages found by the issue's grep command, then read RewriteTest.java, SourceSpec.java, and the Kotlin Assertions implementation from #8557. Compare each language's Assertions entry points and tests, and define completion as unknown/erroneous validation being honored with intentional opt-outs documented and tested; the shared helper and decision items require maintainer input.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp, go, groovy, java, javascript, kotlin, python, ruby, scala, typescript
- Domain
- developer-experience, testing-qa
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100