Migrate `ValidationCode` to PSI #186

yevhenii-nadtochii · 2025-01-22T16:51:23Z

This change set migrates the main Java validation renderer to PSI.

All Java class modifications are now performed within a single method in MessageValidationCode.render(psiFile), allowing for quick grasp of what is actually mutated and how.

This class deals with JavaPoet only in a degree to remain compatible with rules-based CodeGenerator output. These generators produce CodeBlock, MethodSpec and FieldSpec, which are easily converted to PsiStatements (our custom wrapper for PsiCodeBlock, see below), PsiMethod and PsiField.

Note: this PR doesn't bring new features, only migration to PSI and refactoring. Another preparational step to introduce option-specific generators.

Particular changes

Explicitly mentioned in docs to @Validated and @NonValidated that these annotations are not meant for runtime.
Added tests ensuring the generated messages implement ValidatableMessage and ValidatingBuilder interfaces.
CodeGenerator.supportingMemebers() has been split into supportingFields() and supportingMethods(). We can't create general-purpose elements with PSI, only specific ones. Usage of Queriyng were replaced with TypeSystem.
I've made ValidationPlugin abstract, with renderers being expected on the constructor. This makes the declaration of JavaValidationPlugin more straightforward.
MessageValidationCode now contains both modifications of the message class and its builder. I prefer having them both in the same method because they are intertwined. Modifying of Builder.build() doesn't make sense without presence of Message.validate().
Now, we generate only return java.util.Optional.empty(); in validate() when the message doesn't declare constraints. No need to put there an if block, which always evaluates to Optional.empty().
Renamed SetOnceValidationRenderer to SetOnceRenderer. Now, it looks like this Validation word is redundant.

Removed classes

These classes were merged into a single MessageValidationCode:

ImplementValidatingBuilder.
ValidateMethodCode.
ValidationCode.
ValidationConstraintsCode.

These classes are no longer needed (all from java.point package):

BuilderInsertionPoint
BuilderMethodReturnTypeAnnotation
BuildPartialReturnTypeAnnotation.
CachingJavaClassSource.
ExtraValidation.
ParsedSources.
ValidateBeforeReturn.
BuilderMethodsSpec was testing the insertion points, not the generated code.
MockPrinter was a fixture for BuilderMethodsSpec.

API suggestions for PSI

PsiClass.nested(simpleName: String): PsiClass – allows easily get a nested Message.Builder when you already have a PsiClass of the Message at your disposal.
PsiElement.findFirstByText(...), which accepts String. I have taken it from (set_once) implementation because this is the most reliable way to find a place to inject the code somewhere in the method body. Even if we know, that we want to inject before the last statement – it doesn't help. The last PsiElement in PsiMethod.body is always right brace. Ok, let's try its previous sibling (should it be return?) – no, it can be return ...;, whitespace or even line break. Depends on the formatting. PSI stores formatting elements along with other elements.
PsiStatements wrapper to avoid dealing with PsiCodeBlock when we need to represent a list of statements. In validation codebase, we treat code blocks as List<Statement>. But within PSI, a code block is more about Java grammar, where it is a list of statements and the curly braces { <statements> }. This sophisticates 1) creating a set of statements from text and 2) inserting them to other code blocks.
PsiClass.findMethodBySignature() and PsiClass.methodWithSignature(), which were introduced for (set_once).

codecov · 2025-01-23T17:20:04Z

Codecov Report

Attention: Patch coverage is 1.38889% with 142 lines in your changes missing coverage. Please review.

Project coverage is 32.47%. Comparing base (9748a15) to head (20a0d61).
Report is 39 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master     #186      +/-   ##
============================================
+ Coverage     32.16%   32.47%   +0.31%     
+ Complexity      329      312      -17     
============================================
  Files           141      131      -10     
  Lines          2786     2590     -196     
  Branches        227      213      -14     
============================================
- Hits            896      841      -55     
+ Misses         1824     1689     -135     
+ Partials         66       60       -6

yevhenii-nadtochii · 2025-01-24T14:23:00Z

@armiol @alexander-yevsyukov PTAL

alexander-yevsyukov

Please see my comments.

alexander-yevsyukov · 2025-01-24T14:47:28Z

java-runtime/src/main/java/io/spine/validate/NonValidated.java

 * message which is going to be validated later.
 *
+ * <p>Note: this annotation is not intended to be retained at runtime.


This block repeats the same idea: the retention is CLASS because we don't need RUNTIME. Please rephrase.

alexander-yevsyukov · 2025-01-24T14:49:25Z

java-runtime/src/main/java/io/spine/validate/Validated.java

@@ -42,6 +42,11 @@
 * is overridden with a version which returns only valid messages, that version should be marked
 * with this annotation.
 *
+ * <p>Note: this annotation is not intended to be retained at runtime.


This block repeats the same idea: the retention is CLASS because we don't need RUNTIME. Please rephrase.

alexander-yevsyukov · 2025-01-24T14:56:22Z

java/src/main/kotlin/io/spine/validation/java/JavaMessageRenderer.kt

+    /**
+     * Exposes [typeSystem] property, so that the code generation context could use it.
+     */
+    public override val typeSystem: TypeSystem


Let's keep the encapsulation here, as discussed vocally.

alexander-yevsyukov · 2025-01-24T15:25:37Z

java/src/main/kotlin/io/spine/validation/java/JavaValidationPlugin.kt

- *
- * @param base The base plugin to extend.
- * @constructor Creates an instance that contains all the components from the given [base] plugin.
+ * Note: [`(set_once)`][SetOnceRenderer] option is rendered with its own


It's not the option which is rendered. We render validation code serving the (set_once) constraint.

I would start the explanation of the matter with the enumeration of the renderers included into the ValidationPlugin, and then explain them both briefly.

alexander-yevsyukov · 2025-01-24T15:28:02Z

java/src/main/kotlin/io/spine/validation/java/MessageValidationCode.kt

+            |public java.util.Optional<io.spine.validate.ValidationError> validate() {
+            |    ${validateMethodBody(formattedConstraints)}
+            |}
+            """.trimMargin(), this


Why do we use the style which requires trimMargin() here, and the trimIndent() style below?

armiol

@yevhenii-nadtochii LGTM in general. Please see my comments.

armiol · 2025-01-24T16:29:48Z

java/src/main/kotlin/io/spine/validation/java/JavaValidationPlugin.kt

+ * Note: [`(set_once)`][SetOnceRenderer] validation code is rendered with its own
+ * renderer because it significantly differs from the rest of constraints. This option
+ * modifies the message builder behavior. The rest of the constraints are more about
+ * message state assertions.


Did you mean "builder state assertions"?

armiol · 2025-01-24T16:30:18Z

java/src/main/kotlin/io/spine/validation/java/JavaValidationRenderer.kt

- * implement [io.spine.validate.ValidatingBuilder] interface. Logically, it is a part
- * of this renderer, but as for now, it is a standalone renderer before this class
- * migrates to PSI.
+ * This rendered is applied to every [Message] being compiled, even if the message


"This rendered ..." has some grammar issues.

armiol · 2025-01-24T16:30:33Z

java/src/main/kotlin/io/spine/validation/java/MessageValidationCode.kt

@@ -0,0 +1,237 @@
+/*
+ * Copyright 2024, TeamDev. All rights reserved.


armiol · 2025-01-24T16:31:00Z

java/src/main/kotlin/io/spine/validation/java/MessageValidationCode.kt

+ * All validation logic is injected into the [ValidatableMessage.validate] method
+ * within the message class itself. This allows a message instance to be validated even
+ * after it has been built. Note that this method does not throw exceptions directly;
+ * instead, it returns any detected violations, if any.


"...it returns the detected violations, if any."

armiol · 2025-01-24T16:31:26Z

java/src/main/kotlin/io/spine/validation/java/MessageValidationCode.kt

+ * instead, it returns any detected violations, if any.
+ *
+ * The message builder is modified to invoke the [ValidatableMessage.validate] just
+ * before returning from its [build][com.google.protobuf.Message.Builder.build] method.


"before returning the result from ..."

armiol · 2025-01-24T16:33:17Z

java/src/main/kotlin/io/spine/validation/java/MessageValidationCode.kt

+    }
+
+    /**
+     * Generates a Java [constraint][constraints], [supportingFields] and [supportingMethods]


"a" cannot be used with plurals. Please review.

armiol · 2025-01-24T16:35:07Z

java/src/main/kotlin/io/spine/validation/java/MessageValidationCode.kt

+     * 1. [constraints] are joined to [String] using [joinToString] instead of `joinByLine()`
+     * because the contained code blocks already have new lines when converted to string.
+     * 2. For the same reason, we trim the result because a trailing empty line is not needed.
+     * 3. When creating PSI method from the text, we have to use [trimMargin] and '|' symbols


Please review this p.3 using any grammar checker. I personally use Grazie (as a Chrome extension).

I suspect some prepositions are missing.

armiol · 2025-01-24T16:36:08Z

java/src/main/kotlin/io/spine/validation/java/psi/PsiClassExts.kt

- * Searches for a method in this [PsiClass] matching the given signature
+ * Returns a nested class declared in this [PsiClass].
+ *
+ * @param simpleName The class simple name.


"The simple name of the class."

armiol · 2025-01-24T16:38:30Z

model/src/main/kotlin/io/spine/validation/ValidationPlugin.kt

+ * The basic implementation of ProtoData validation plugin, which builds
+ * language-agnostic representation of the declared constraints.
+ *
+ * The concrete implementations should provide [renderers], which will


I would remove "will".

yevhenii-nadtochii · 2025-01-24T17:30:34Z

@armiol @alexander-yevsyukov PTAL

alexander-yevsyukov

LGTM

yevhenii-nadtochii added 7 commits January 22, 2025 10:15

Document ValidationCode

2bbb61d

Implement ValidatableMessage interface with PSI

9e2f236

Split supportingMembers() into fields and methods members

9c6dbdf

Use PSI to add validate() method

465fff8

Migrate ValidationCode to PSI

139b0cc

Inline ValidateMethodCode into ValidationCode

946a7e8

Migrate builder method annotating to PSI

4d4bf89

yevhenii-nadtochii self-assigned this Jan 22, 2025

yevhenii-nadtochii added 15 commits January 23, 2025 12:48

Use deepSearch() to find the statement

b8444ac

Remove no longer used insertion points

3df1b0e

Implement ValidatingBuilder within the main renderer

64dafcb

Remove no longer used classes

207c784

Simplify JavaValidationPlugin definition

1aae3ad

Add tests for annotations

7bdb13d

Merge branch 'test-annotations-and-interfaces' into migrate-to-PSI

c1b9602

Move potential PSI extensions to a separate file

811c0ed

Add PsiClass.nested() extension

603413c

Return deepSearch() to extensions

a8811a4

Do not cast right in extension

a9a4598

Extract BuilderValidationCode

555cad3

Try out more domain names for renderers

cfcb73d

Bump the version -> 2.0.0-SNAPSHOT.190

c822b2c

Update reports

e9def96

yevhenii-nadtochii added 6 commits January 24, 2025 11:22

Add notes about the annotation retention policy

3e38190

Remove tests for semantic-only annotations

574ad1a

Document MessageValidationCode

cc57280

Proofread docs

60ce73a

Update docs to ValidationPlugin and its implementation

a31a138

Rename a method

e03a6e6

Proofread docs to PsiStatements

f5afcc6

yevhenii-nadtochii marked this pull request as ready for review January 24, 2025 14:22

yevhenii-nadtochii requested review from armiol and alexander-yevsyukov January 24, 2025 14:23

yevhenii-nadtochii added 4 commits January 24, 2025 15:51

Roll back JavaValidationRenderer

7fde471

Avoid duplicated sentences

9db4232

Replace usages of Querying with TypeSystem in the context

196afeb

Rename deepSearch() to findFirstByText()

738d92c

alexander-yevsyukov requested changes Jan 24, 2025

View reviewed changes

yevhenii-nadtochii added 2 commits January 24, 2025 16:39

Rephrase the sentence

a20722f

Document formatting peculiarities in declareValidateMethod()

c0c9c7a

armiol requested changes Jan 24, 2025

View reviewed changes

yevhenii-nadtochii added 3 commits January 24, 2025 17:57

Remove ambiguous wording

14959d4

Proofread docs

4a169ac

Expand docs about each renderer

20a0d61

yevhenii-nadtochii requested review from armiol and alexander-yevsyukov January 24, 2025 17:30

armiol approved these changes Jan 24, 2025

View reviewed changes

alexander-yevsyukov approved these changes Jan 24, 2025

View reviewed changes

yevhenii-nadtochii merged commit d0f1c9b into master Jan 24, 2025
7 checks passed

yevhenii-nadtochii deleted the migrate-to-PSI branch January 24, 2025 17:46

		@@ -0,0 +1,237 @@
		/*
		* Copyright 2024, TeamDev. All rights reserved.

Migrate ValidationCode to PSI #186

Migrate ValidationCode to PSI #186

Uh oh!

Conversation

yevhenii-nadtochii commented Jan 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Particular changes

Removed classes

API suggestions for PSI

Uh oh!

codecov bot commented Jan 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

yevhenii-nadtochii commented Jan 24, 2025

Uh oh!

alexander-yevsyukov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

armiol left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yevhenii-nadtochii commented Jan 24, 2025

Uh oh!

alexander-yevsyukov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Migrate `ValidationCode` to PSI #186

Migrate `ValidationCode` to PSI #186

yevhenii-nadtochii commented Jan 22, 2025 •

edited

Loading

codecov bot commented Jan 23, 2025 •

edited

Loading