JSON: Provide semantic analyser to improve rendering

[matthiasblaesing]

This change aligns JSON rendering with JavaScript rendering for object literals. At its core it ensures, that the property names are rendered in a different color than the values. In the screenshot the left side shows the old style, on the right side the new rendering is visible (using the "FlatLaf Light" editor style).

[mbien]

question: how does this work? JS comment blocks in a json?

[matthiasblaesing]

Sure - it is a web-era format. Of course they don't follow a specification. An example from a freshly generated angular project:

/* To learn more about Typescript configuration file: https://www.typescriptlang.org/docs/handbook/tsconfig-json.html. */
/* To learn more about Angular compiler options: https://angular.dev/reference/configs/angular-compiler-options. */
{
  "extends": "./tsconfig.json",
  "compilerOptions": {
    "outDir": "./out-tsc/app",
    "types": []
  },
  "files": [
    "src/main.ts"
  ],
  "include": [
    "src/**/*.d.ts"
  ]
}

Not sure which side is wrong here. The short sighted specification or the specification ignoring usage.

[mbien]

interesting, thanks!

performance wise:
You have more experience there, but if someone would edit huge json files which have comments, this could be a performance problem. Since this scales currently with prop_count * comment_count. Not sure what the keys of the map are (Integer for line number?, offset?).

It might be also worth to move JsDocumentationSupport.getDocumentationHolder(result).getCommentBlocks() out of the outer for loop. I had a similar case #8525 (comment) where moving it out made the inner loop 10x faster in large files (without hurting readability) since it allowed the JVM to inline everything.

something like:

diff --git a/webcommon/javascript2.editor/src/org/netbeans/modules/javascript2/editor/JsonSemanticAnalyzer.java b/webcommon/javascript2.editor/src/org/netbeans/modules/javascript2/editor/JsonSemanticAnalyzer.java
index 965f618..d71fde0 100644
--- a/webcommon/javascript2.editor/src/org/netbeans/modules/javascript2/editor/JsonSemanticAnalyzer.java
+++ b/webcommon/javascript2.editor/src/org/netbeans/modules/javascript2/editor/JsonSemanticAnalyzer.java
@@ -75,17 +75,20 @@
         if (ModelUtils.wasProcessed(parent, processedObjects)) {
             return highlights;
         }
+        
+        Map<Integer, ? extends JsComment> comments = JsDocumentationSupport.getDocumentationHolder(result).getCommentBlocks();
+        
         for (Iterator<? extends JsObject> it = parent.getProperties().values().iterator(); it.hasNext();) {
             JsObject object = it.next();
             if (object.getDeclarationName() != null) {
                 switch (object.getJSKind()) {
                     case OBJECT_LITERAL:
                         if(object.getDeclarationName() != null) {
-                            addColoring(result, highlights, object.getDeclarationName().getOffsetRange(), ColoringAttributes.FIELD_SET);
+                            addColoring(result, comments, highlights, object.getDeclarationName().getOffsetRange(), ColoringAttributes.FIELD_SET);
                         }
                         break;
                     case PROPERTY:
-                        addColoring(result, highlights, object.getDeclarationName().getOffsetRange(), ColoringAttributes.FIELD_SET);
+                        addColoring(result, comments, highlights, object.getDeclarationName().getOffsetRange(), ColoringAttributes.FIELD_SET);
                         break;
                 }
             }
@@ -101,17 +104,17 @@
         return highlights;
     }
 
-    private void addColoring(JsonParserResult result, Map<OffsetRange, Set<ColoringAttributes>> highlights, OffsetRange astRange, Set<ColoringAttributes> coloring) {
+    private void addColoring(JsonParserResult result, Map<Integer, ? extends JsComment> comments, Map<OffsetRange, Set<ColoringAttributes>> highlights, OffsetRange astRange, Set<ColoringAttributes> coloring) {
         int start = result.getSnapshot().getOriginalOffset(astRange.getStart());
         int end = result.getSnapshot().getOriginalOffset(astRange.getEnd());
-        if (start > -1 && end > -1 && start < end && !isInComment(result, astRange)) {
+        if (start > -1 && end > -1 && start < end && !isInComment(result, comments, astRange)) {
             OffsetRange range = start == astRange.getStart() ? astRange : new OffsetRange(start, end);
             highlights.put(range, coloring);
         }
     }
 
-    private boolean isInComment(JsonParserResult result, OffsetRange range) {
-        for (JsComment comment : JsDocumentationSupport.getDocumentationHolder(result).getCommentBlocks().values()) {
+    private boolean isInComment(JsonParserResult result, Map<Integer, ? extends JsComment> comments, OffsetRange range) {
+        for (JsComment comment : comments.values()) {
             if (comment.getOffsetRange().containsInclusive(range.getStart())) {
                 return true;
             }

[mbien]

btw I might be doing something wrong, but I used the json file you posted as test, set a breakpoint and getCommentBlocks() was always empty.

[matthiasblaesing]

Ok, checked again. As far as I can see, comments never reach the parser level and thus will not be relevant for analysis. I dropped the part (will squash with first comment).

[matthiasblaesing]

@mbien followed your suggestion. JsonSemanticAnalyser is a cut down copy of JsSemanticAnalyser, so I looked there also and from there fixed some more things. Pushed update as a separate commit.

[matthiasblaesing]

Could we please not open X side quests for a trivial change?

[matthiasblaesing]

If there are no objections, I'll squash and merge by the end of the week.

[lkishalmi]

Just out of curiosity, what was the purpose of these removed Translators?

[matthiasblaesing]

My understanding is, that the translators are actually embedding providers, that used various techniques to extract JS out of different source file formats. The removed translators are actually not referenced, for example the xhtml processor was move to the html.editor module.

[lkishalmi]

Looks good to me!

[matthiasblaesing]

@lkishalmi @mbien thank you both for review. Lets get this in.