Adding missing @Override annotations (part 3). Adding RRA/RWA/RUI annotations. Some localization and refactoring by unv-unv · Pull Request #13 · consulo/consulo-spring

unv-unv · 2026-07-25T10:20:54Z

No description provided.

…otations. Some localization and refactoring.

unv-unv · 2026-07-25T10:21:19Z

@claude review pr with inline comments

github-actions · 2026-07-25T10:21:37Z

Claude finished @unv-unv's task in 4m 33s —— View job

PR Review — @OverRide / RRA-RWA-RUI annotations, localization, refactoring

Read repo guidelines (no AGENTS.md/CLAUDE.md present)
Analyze the PR diff (62 files, +994/−710)
Post inline review comments with suggestions

I reviewed all 62 changed files. The bulk of this PR is clean, mechanical, and looks good: adding missing @Override, @RequiredReadAction/@RequiredWriteAction/@RequiredUIAccess, SpringBundle→SpringLocalize / IdeBundle→IdeLocalize migration, SpringIcons→SpringImplIconGroup, pattern-matching instanceof, diamond operators, Objects.equals/hashCode, and lambda conversions. Nice cleanups like the FIELD_RETRIVING→FIELD_RETRIEVING / checkbeanClass→checkBeanClass typo fixes too.

I left 3 inline comments — one worth a real look, two minor:

⚠️ CustomBeanRegistry escaping change (discussion) — StringUtil.escapeStringCharacters → StringEscapeUtil.escape(text, '"') on the line sent to the out-of-process parser. That parser reads with readLine() and decode()s \n/\r/\t/\uXXXX; if the new escape() no longer converts newlines/control chars, multi-line <bean> tags will break the line-framed protocol. Same compatibility question for the paired StringEscapeUtil.unescape(...) calls. Please verify the escape/unescape set matches the parser's encode/decode — this is the one item I'd confirm before merge.
DeclareParentsInspection.registerProblem (discussion) — the explicit ProblemHighlightType.GENERIC_ERROR_OR_WARNING is dropped; confirm newProblemDescriptor(...).create() defaults to the same highlight type.
DependencyCheck.fromDefault (discussion) — minor: the default -> DEFAULT branch is now unreachable (switch is exhaustive with case null + all constants); optional suggestion to drop it.

Everything else looks good to me. 👍
· overrides-3

github-actions · 2026-07-25T10:25:53Z


    writer.println("input");
-    writer.println(StringUtil.escapeStringCharacters(tag.getText()));
+    writer.println(StringEscapeUtil.escape(tag.getText(), '"'));


⚠️ Please verify this escaping change is protocol-compatible.

The custom-bean text is sent to the out-of-process parser on a single line (writer.println(...)), and the parser reads it back with input.readLine() and its own decode() — which un-escapes \n, \r, \t, \b, \f, \", \', \\ and \uXXXX (see CustomBeanParser.decode()).

The previous StringUtil.escapeStringCharacters(tag.getText()) escaped newlines/control chars, so a multi-line <bean> tag was collapsed into one line that survives readLine(). If StringEscapeUtil.escape(text, '"') only escapes " (and \) and leaves real newlines intact, then every multi-line tag — i.e. essentially all of them — will break the readLine() framing and parsing will fail.

The mirror-image concern applies to the two StringEscapeUtil.unescape(...) calls (lines 411 and 433), which now decode the parser's encode() output. Please confirm escape/unescape handle the same escape set as the parser's encode/decode; if not, keeping StringUtil.escapeStringCharacters / StringUtil.unescapeStringCharacters here would be safer.

github-actions · 2026-07-25T10:25:58Z

-            descriptionTemplate,
-            ProblemHighlightType.GENERIC_ERROR_OR_WARNING
-        ));
+        holder.registerProblem(holder.getManager().newProblemDescriptor(descriptionTemplate).range(element, range).create());


The old implementation passed ProblemHighlightType.GENERIC_ERROR_OR_WARNING explicitly. newProblemDescriptor(...).range(...).create() relies on the builder's default highlight type. Please confirm that default is still GENERIC_ERROR_OR_WARNING, otherwise the severity/appearance of this reported problem changes silently.

github-actions · 2026-07-25T10:25:59Z

+            case NONE -> NONE;
+            case OBJECTS -> OBJECTS;
+            case SIMPLE -> SIMPLE;
+            default -> DEFAULT;


Minor: case null plus all four DefaultDependencyCheck constants already make this switch exhaustive, so this default -> DEFAULT branch is unreachable. Dropping it turns any future addition to DefaultDependencyCheck into a compile error you must handle explicitly (rather than silently mapping it to DEFAULT):

Suggested change

default -> DEFAULT;

unv-unv · 2026-07-25T10:50:32Z

@claude review pr with inline comments

github-actions · 2026-07-25T10:50:53Z

Claude finished @unv-unv's task in 4m 39s —— View job

PR Review — @OverRide / RRA-RWA-RUI annotations, localization, refactoring

Read repo guidelines & gather PR context
Analyze the full diff (63 files)
Verify the CustomBeanRegistry escaping change against the parser protocol
Post inline review comments with suggestions

The bulk of this PR is clean and mechanical — @Override, @RequiredReadAction/@RequiredWriteAction/@RequiredUIAccess, SpringBundle→SpringLocalize, icon-group migration, pattern-matching instanceof, diamonds, lambdas, Objects.equals/hashCode, and typo fixes. That all looks good. 👍

I re-checked the three items from the earlier pass and traced the parser protocol to a firm conclusion this time:

⚠️ 1. `CustomBeanRegistry` escaping change — confirmed protocol-breaking regression (3 inline comments)

Following the round-trip through the out-of-process CustomBeanParser (encode()/decode()), the swap from StringUtil.escapeStringCharacters/unescapeStringCharacters to StringEscapeUtil.escape/unescape is not protocol-compatible:

Send side (line 488): the tag goes out via writer.println(...) and the parser reads it with input.readLine(). StringEscapeUtil.escape(text, '"') only escapes "/\ and leaves real newlines intact, so any multi-line <bean> tag now spans multiple physical lines and readLine() truncates it → parsing fails. The old escapeStringCharacters collapsed newlines to \n, matching the parser's decode().
Receive side (line 412, line 434): StringEscapeUtil.unescape only strips backslashes; it does not decode \n→newline or \uXXXX→char. The parser emits those via encode(), so exception text and property values with newlines/control chars get corrupted.

Each inline comment has a one-click suggestion to revert to StringUtil.escapeStringCharacters / unescapeStringCharacters. This is the one item I'd fix before merge.

2. `DeclareParentsInspection.registerProblem` — minor confirm (inline)

The explicit ProblemHighlightType.GENERIC_ERROR_OR_WARNING is dropped; newProblemDescriptor(...).create() now relies on the builder default. Please confirm that default is still GENERIC_ERROR_OR_WARNING.

3. `DependencyCheck.fromDefault` — ✅ already resolved

The current diff already drops the unreachable default -> DEFAULT branch; the switch is now exhaustive (case null + all four DefaultDependencyCheck constants, which are exactly ALL/NONE/OBJECTS/SIMPLE). Nothing to do.

Everything else looks good to me.
· overrides-3

github-actions · 2026-07-25T10:54:22Z


    writer.println("input");
-    writer.println(StringUtil.escapeStringCharacters(tag.getText()));
+    writer.println(StringEscapeUtil.escape(tag.getText(), '"'));


@.claude-review-c488.md

github-actions · 2026-07-25T10:54:42Z

          if ("exception".equals(first)) {
            assert lines.length == k + 2;
-            setResult(new ParseResult(StringUtil.unescapeStringCharacters(lines[k + 1]), true));
+            setResult(new ParseResult(StringEscapeUtil.unescape(lines[k + 1]), true));


@.claude-review-c412.md

github-actions · 2026-07-25T10:54:43Z

-                @NonNls String prop = lines[j++];
-                @NonNls String propValue = StringUtil.unescapeStringCharacters(lines[j++]);
+                String prop = lines[j++];
+                String propValue = StringEscapeUtil.unescape(lines[j++]);


@.claude-review-c434.md

github-actions · 2026-07-25T10:54:58Z

-            descriptionTemplate,
-            ProblemHighlightType.GENERIC_ERROR_OR_WARNING
-        ));
+        holder.registerProblem(holder.getManager().newProblemDescriptor(descriptionTemplate).range(element, range).create());


@.claude-review-dp.md

Adding missing @OverRide annotations (part 3). Adding RRA/RWA/RUI ann…

32126f7

…otations. Some localization and refactoring.

github-actions Bot reviewed Jul 25, 2026

View reviewed changes

unv-unv added 2 commits July 25, 2026 13:32

Fixes.

19fc572

Fixing compilation of SpringBeanIdConverter.

0ea06f2

github-actions Bot reviewed Jul 25, 2026

View reviewed changes

VISTALL merged commit 6b29825 into consulo:master Jul 25, 2026
1 check passed

unv-unv deleted the overrides-3 branch July 26, 2026 12:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Adding missing @Override annotations (part 3). Adding RRA/RWA/RUI annotations. Some localization and refactoring - #13

Adding missing @Override annotations (part 3). Adding RRA/RWA/RUI annotations. Some localization and refactoring#13
VISTALL merged 3 commits into
consulo:masterfrom
unv-unv:overrides-3

unv-unv commented Jul 25, 2026

Uh oh!

unv-unv commented Jul 25, 2026

Uh oh!

github-actions Bot commented Jul 25, 2026 •

edited

Loading

Uh oh!

github-actions Bot Jul 25, 2026

Uh oh!

github-actions Bot Jul 25, 2026

Uh oh!

github-actions Bot Jul 25, 2026

Uh oh!

unv-unv commented Jul 25, 2026

Uh oh!

github-actions Bot commented Jul 25, 2026 •

edited

Loading

Uh oh!

github-actions Bot Jul 25, 2026

Uh oh!

github-actions Bot Jul 25, 2026

Uh oh!

github-actions Bot Jul 25, 2026

Uh oh!

github-actions Bot Jul 25, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

unv-unv commented Jul 25, 2026

Uh oh!

unv-unv commented Jul 25, 2026

Uh oh!

github-actions Bot commented Jul 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review — @OverRide / RRA-RWA-RUI annotations, localization, refactoring

Uh oh!

github-actions Bot Jul 25, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 25, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 25, 2026

Choose a reason for hiding this comment

Uh oh!

unv-unv commented Jul 25, 2026

Uh oh!

github-actions Bot commented Jul 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review — @OverRide / RRA-RWA-RUI annotations, localization, refactoring

⚠️ 1. CustomBeanRegistry escaping change — confirmed protocol-breaking regression (3 inline comments)

2. DeclareParentsInspection.registerProblem — minor confirm (inline)

3. DependencyCheck.fromDefault — ✅ already resolved

Uh oh!

github-actions Bot Jul 25, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 25, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 25, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 25, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented Jul 25, 2026 •

edited

Loading

github-actions Bot commented Jul 25, 2026 •

edited

Loading

⚠️ 1. `CustomBeanRegistry` escaping change — confirmed protocol-breaking regression (3 inline comments)

2. `DeclareParentsInspection.registerProblem` — minor confirm (inline)

3. `DependencyCheck.fromDefault` — ✅ already resolved