You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by GitBox <gi...@apache.org> on 2020/10/16 05:02:59 UTC

[GitHub] [netbeans] jlahoda commented on a change in pull request #2444: [NETBEANS-3588] Code Templates not working in Java Editor in for loops

jlahoda commented on a change in pull request #2444:
URL: https://github.com/apache/netbeans/pull/2444#discussion_r505180243



##########
File path: java/java.editor/src/org/netbeans/modules/editor/java/JavaCodeTemplateFilter.java
##########
@@ -72,88 +73,125 @@ private JavaCodeTemplateFilter(JTextComponent component, int offset) {
             final Source source = Source.create(component.getDocument());
             if (source != null) {
                 final AtomicBoolean cancel = new AtomicBoolean();
-                ProgressUtils.runOffEventDispatchThread(new Runnable() {
-                    @Override
-                    public void run() {
-                        try {
-                            ParserManager.parse(Collections.singleton(source), new UserTask() {
-                                @Override
-                                public void run(ResultIterator resultIterator) throws Exception {
-                                    if (cancel.get()) {
-                                        return;
+                BaseProgressUtils.runOffEventDispatchThread(() -> {
+                    try {
+                        ParserManager.parse(Collections.singleton(source), new UserTask() {
+                            @Override
+                            public void run(ResultIterator resultIterator) throws Exception {
+                                if (cancel.get()) {
+                                    return;
+                                }
+                                Parser.Result result = resultIterator.getParserResult(startOffset);
+                                CompilationController controller = result != null ? CompilationController.get(result) : null;
+                                if (controller != null && Phase.PARSED.compareTo(controller.toPhase(Phase.PARSED)) <= 0) {
+                                    TreeUtilities tu = controller.getTreeUtilities();
+                                    int eo = endOffset;
+                                    int so = startOffset;
+                                    if (so >= 0) {
+                                        so = result.getSnapshot().getEmbeddedOffset(startOffset);
                                     }
-                                    Parser.Result result = resultIterator.getParserResult(startOffset);
-                                    CompilationController controller = result != null ? CompilationController.get(result) : null;
-                                    if (controller != null && Phase.PARSED.compareTo(controller.toPhase(Phase.PARSED)) <= 0) {
-                                        TreeUtilities tu = controller.getTreeUtilities();
-                                        int eo = endOffset;
-                                        int so = startOffset;
-                                        if (so >= 0) {
-                                            so = result.getSnapshot().getEmbeddedOffset(startOffset);
-                                        }
-                                        if (endOffset >= 0) {
-                                            eo = result.getSnapshot().getEmbeddedOffset(endOffset);
-                                            TokenSequence<JavaTokenId> ts = SourceUtils.getJavaTokenSequence(controller.getTokenHierarchy(), so);
-                                            int delta = ts.move(so);
+                                    if (endOffset >= 0) {
+                                        eo = result.getSnapshot().getEmbeddedOffset(endOffset);
+                                        TokenSequence<JavaTokenId> ts = SourceUtils.getJavaTokenSequence(controller.getTokenHierarchy(), so);
+                                        int delta = ts.move(so);
+                                        if (delta == 0 || ts.moveNext() && ts.token().id() == JavaTokenId.WHITESPACE) {
+                                            delta = ts.move(eo);
                                             if (delta == 0 || ts.moveNext() && ts.token().id() == JavaTokenId.WHITESPACE) {
-                                                delta = ts.move(eo);
-                                                if (delta == 0 || ts.moveNext() && ts.token().id() == JavaTokenId.WHITESPACE) {
-                                                    String selectedText = controller.getText().substring(so, eo).trim();
-                                                    SourcePositions[] sp = new SourcePositions[1];
-                                                    ExpressionTree expr = selectedText.length() > 0 ? tu.parseExpression(selectedText, sp) : null;
-                                                    if (expr != null && expr.getKind() != Tree.Kind.IDENTIFIER && !Utilities.containErrors(expr) && sp[0].getEndPosition(null, expr) >= selectedText.length()) {
-                                                        stringCtx = EXPRESSION;
-                                                    }
+                                                String selectedText = controller.getText().substring(so, eo).trim();
+                                                SourcePositions[] sp = new SourcePositions[1];
+                                                ExpressionTree expr = selectedText.length() > 0 ? tu.parseExpression(selectedText, sp) : null;
+                                                if (expr != null && expr.getKind() != Tree.Kind.IDENTIFIER && !Utilities.containErrors(expr) && sp[0].getEndPosition(null, expr) >= selectedText.length()) {
+                                                    stringCtx = EXPRESSION;
                                                 }
                                             }
                                         }
-                                        Tree tree = tu.pathFor(so).getLeaf();
-                                        if (eo >= 0 && so != eo) {
-                                            if (tu.pathFor(eo).getLeaf() != tree) {
-                                                return;
-                                            }
+                                    }
+                                    Tree tree = tu.pathFor(so).getLeaf();
+                                    if (eo >= 0 && so != eo) {
+                                        if (tu.pathFor(eo).getLeaf() != tree) {
+                                            return;
                                         }
-                                        treeKindCtx = tree.getKind();
-                                        switch (treeKindCtx) {
-                                            case CASE:
-                                                if (so < controller.getTrees().getSourcePositions().getEndPosition(controller.getCompilationUnit(), ((CaseTree)tree).getExpression())) {
-                                                    treeKindCtx = null;
-                                                }
-                                                break;
-                                            case CLASS:
-                                                SourcePositions sp = controller.getTrees().getSourcePositions();
-                                                int startPos = (int)sp.getEndPosition(controller.getCompilationUnit(), ((ClassTree)tree).getModifiers());
-                                                if (startPos <= 0) {
-                                                    startPos = (int)sp.getStartPosition(controller.getCompilationUnit(), tree);
-                                                }
-                                                String headerText = controller.getText().substring(startPos, so);
-                                                int idx = headerText.indexOf('{'); //NOI18N
-                                                if (idx < 0) {
-                                                    treeKindCtx = null;
-                                                    stringCtx = CLASS_HEADER;
-                                                }
-                                                break;
-                                            case FOR_LOOP:
-                                            case ENHANCED_FOR_LOOP:
-                                            case WHILE_LOOP:
-                                                sp = controller.getTrees().getSourcePositions();
+                                    }
+                                    treeKindCtx = tree.getKind();
+                                    switch (treeKindCtx) {
+                                        case CASE:
+                                            if (so < controller.getTrees().getSourcePositions().getEndPosition(controller.getCompilationUnit(), ((CaseTree)tree).getExpression())) {
+                                                treeKindCtx = null;
+                                            }
+                                            break;
+                                        case CLASS:
+                                            SourcePositions sp = controller.getTrees().getSourcePositions();
+                                            int startPos = (int)sp.getEndPosition(controller.getCompilationUnit(), ((ClassTree)tree).getModifiers());
+                                            if (startPos <= 0) {
                                                 startPos = (int)sp.getStartPosition(controller.getCompilationUnit(), tree);
-                                                String text = controller.getText().substring(startPos, so);
-                                                if (!text.trim().endsWith(")")) {
-                                                    treeKindCtx = null;
+                                            }
+                                            String headerText = controller.getText().substring(startPos, so);
+                                            int idx = headerText.indexOf('{'); //NOI18N
+                                            if (idx < 0) {
+                                                treeKindCtx = null;
+                                                stringCtx = CLASS_HEADER;
+                                            }
+                                            break;
+                                        case FOR_LOOP:
+                                        case ENHANCED_FOR_LOOP:
+                                            if (!isRightParenthesisOfLoopPresent(component, controller)) {
+                                                 treeKindCtx = null;
+                                            }
+                                            break;
+                                        case PARENTHESIZED:
+                                            if (isPartOfWhileLoop(component, controller)) {
+                                                if (!isRightParenthesisOfLoopPresent(component, controller)) {
+                                                        treeKindCtx = null;
                                                 }
-                                        }
+                                            }
+                                            break;
                                     }
                                 }
-                            });
-                        } catch (ParseException ex) {
-                            Exceptions.printStackTrace(ex);
-                        }
+                            }
+                        });
+                    } catch (ParseException ex) {
+                        Exceptions.printStackTrace(ex);
                     }
                 }, NbBundle.getMessage(JavaCodeTemplateProcessor.class, "JCT-init"), cancel, false); //NOI18N
             }
         }
     }
+    
+    private boolean isPartOfWhileLoop(JTextComponent component, CompilationController controller) {
+        TreeUtilities treeUtilities = controller.getTreeUtilities();
+        TreePath currentPath = treeUtilities.pathFor(component.getCaretPosition());
+        TreePath parentPath = treeUtilities.getPathElementOfKind(Tree.Kind.WHILE_LOOP, currentPath);
+        return parentPath != null;
+    }
+    
+    private boolean isRightParenthesisOfLoopPresent(JTextComponent component, CompilationController controller) {
+        AtomicBoolean result = new AtomicBoolean(true);
+        Document document = component.getDocument();
+        document.render(() -> {
+            TokenHierarchy<?> tokenHierarchy = controller.getTokenHierarchy();

Review comment:
       CompilationInfo.getTokenHierarchy() is, as far as I know, based on an immutable copy of the Document's content. No need to Document.render.

##########
File path: java/java.editor/src/org/netbeans/modules/editor/java/JavaCodeTemplateFilter.java
##########
@@ -72,88 +73,125 @@ private JavaCodeTemplateFilter(JTextComponent component, int offset) {
             final Source source = Source.create(component.getDocument());
             if (source != null) {
                 final AtomicBoolean cancel = new AtomicBoolean();
-                ProgressUtils.runOffEventDispatchThread(new Runnable() {
-                    @Override
-                    public void run() {
-                        try {
-                            ParserManager.parse(Collections.singleton(source), new UserTask() {
-                                @Override
-                                public void run(ResultIterator resultIterator) throws Exception {
-                                    if (cancel.get()) {
-                                        return;
+                BaseProgressUtils.runOffEventDispatchThread(() -> {
+                    try {
+                        ParserManager.parse(Collections.singleton(source), new UserTask() {
+                            @Override
+                            public void run(ResultIterator resultIterator) throws Exception {
+                                if (cancel.get()) {
+                                    return;
+                                }
+                                Parser.Result result = resultIterator.getParserResult(startOffset);
+                                CompilationController controller = result != null ? CompilationController.get(result) : null;
+                                if (controller != null && Phase.PARSED.compareTo(controller.toPhase(Phase.PARSED)) <= 0) {
+                                    TreeUtilities tu = controller.getTreeUtilities();
+                                    int eo = endOffset;
+                                    int so = startOffset;
+                                    if (so >= 0) {
+                                        so = result.getSnapshot().getEmbeddedOffset(startOffset);
                                     }
-                                    Parser.Result result = resultIterator.getParserResult(startOffset);
-                                    CompilationController controller = result != null ? CompilationController.get(result) : null;
-                                    if (controller != null && Phase.PARSED.compareTo(controller.toPhase(Phase.PARSED)) <= 0) {
-                                        TreeUtilities tu = controller.getTreeUtilities();
-                                        int eo = endOffset;
-                                        int so = startOffset;
-                                        if (so >= 0) {
-                                            so = result.getSnapshot().getEmbeddedOffset(startOffset);
-                                        }
-                                        if (endOffset >= 0) {
-                                            eo = result.getSnapshot().getEmbeddedOffset(endOffset);
-                                            TokenSequence<JavaTokenId> ts = SourceUtils.getJavaTokenSequence(controller.getTokenHierarchy(), so);
-                                            int delta = ts.move(so);
+                                    if (endOffset >= 0) {
+                                        eo = result.getSnapshot().getEmbeddedOffset(endOffset);
+                                        TokenSequence<JavaTokenId> ts = SourceUtils.getJavaTokenSequence(controller.getTokenHierarchy(), so);
+                                        int delta = ts.move(so);
+                                        if (delta == 0 || ts.moveNext() && ts.token().id() == JavaTokenId.WHITESPACE) {
+                                            delta = ts.move(eo);
                                             if (delta == 0 || ts.moveNext() && ts.token().id() == JavaTokenId.WHITESPACE) {
-                                                delta = ts.move(eo);
-                                                if (delta == 0 || ts.moveNext() && ts.token().id() == JavaTokenId.WHITESPACE) {
-                                                    String selectedText = controller.getText().substring(so, eo).trim();
-                                                    SourcePositions[] sp = new SourcePositions[1];
-                                                    ExpressionTree expr = selectedText.length() > 0 ? tu.parseExpression(selectedText, sp) : null;
-                                                    if (expr != null && expr.getKind() != Tree.Kind.IDENTIFIER && !Utilities.containErrors(expr) && sp[0].getEndPosition(null, expr) >= selectedText.length()) {
-                                                        stringCtx = EXPRESSION;
-                                                    }
+                                                String selectedText = controller.getText().substring(so, eo).trim();
+                                                SourcePositions[] sp = new SourcePositions[1];
+                                                ExpressionTree expr = selectedText.length() > 0 ? tu.parseExpression(selectedText, sp) : null;
+                                                if (expr != null && expr.getKind() != Tree.Kind.IDENTIFIER && !Utilities.containErrors(expr) && sp[0].getEndPosition(null, expr) >= selectedText.length()) {
+                                                    stringCtx = EXPRESSION;
                                                 }
                                             }
                                         }
-                                        Tree tree = tu.pathFor(so).getLeaf();
-                                        if (eo >= 0 && so != eo) {
-                                            if (tu.pathFor(eo).getLeaf() != tree) {
-                                                return;
-                                            }
+                                    }
+                                    Tree tree = tu.pathFor(so).getLeaf();
+                                    if (eo >= 0 && so != eo) {
+                                        if (tu.pathFor(eo).getLeaf() != tree) {
+                                            return;
                                         }
-                                        treeKindCtx = tree.getKind();
-                                        switch (treeKindCtx) {
-                                            case CASE:
-                                                if (so < controller.getTrees().getSourcePositions().getEndPosition(controller.getCompilationUnit(), ((CaseTree)tree).getExpression())) {
-                                                    treeKindCtx = null;
-                                                }
-                                                break;
-                                            case CLASS:
-                                                SourcePositions sp = controller.getTrees().getSourcePositions();
-                                                int startPos = (int)sp.getEndPosition(controller.getCompilationUnit(), ((ClassTree)tree).getModifiers());
-                                                if (startPos <= 0) {
-                                                    startPos = (int)sp.getStartPosition(controller.getCompilationUnit(), tree);
-                                                }
-                                                String headerText = controller.getText().substring(startPos, so);
-                                                int idx = headerText.indexOf('{'); //NOI18N
-                                                if (idx < 0) {
-                                                    treeKindCtx = null;
-                                                    stringCtx = CLASS_HEADER;
-                                                }
-                                                break;
-                                            case FOR_LOOP:
-                                            case ENHANCED_FOR_LOOP:
-                                            case WHILE_LOOP:
-                                                sp = controller.getTrees().getSourcePositions();
+                                    }
+                                    treeKindCtx = tree.getKind();
+                                    switch (treeKindCtx) {
+                                        case CASE:
+                                            if (so < controller.getTrees().getSourcePositions().getEndPosition(controller.getCompilationUnit(), ((CaseTree)tree).getExpression())) {
+                                                treeKindCtx = null;
+                                            }
+                                            break;
+                                        case CLASS:
+                                            SourcePositions sp = controller.getTrees().getSourcePositions();
+                                            int startPos = (int)sp.getEndPosition(controller.getCompilationUnit(), ((ClassTree)tree).getModifiers());
+                                            if (startPos <= 0) {
                                                 startPos = (int)sp.getStartPosition(controller.getCompilationUnit(), tree);
-                                                String text = controller.getText().substring(startPos, so);
-                                                if (!text.trim().endsWith(")")) {
-                                                    treeKindCtx = null;
+                                            }
+                                            String headerText = controller.getText().substring(startPos, so);
+                                            int idx = headerText.indexOf('{'); //NOI18N
+                                            if (idx < 0) {
+                                                treeKindCtx = null;
+                                                stringCtx = CLASS_HEADER;
+                                            }
+                                            break;
+                                        case FOR_LOOP:
+                                        case ENHANCED_FOR_LOOP:
+                                            if (!isRightParenthesisOfLoopPresent(component, controller)) {
+                                                 treeKindCtx = null;
+                                            }
+                                            break;
+                                        case PARENTHESIZED:
+                                            if (isPartOfWhileLoop(component, controller)) {
+                                                if (!isRightParenthesisOfLoopPresent(component, controller)) {
+                                                        treeKindCtx = null;
                                                 }
-                                        }
+                                            }
+                                            break;
                                     }
                                 }
-                            });
-                        } catch (ParseException ex) {
-                            Exceptions.printStackTrace(ex);
-                        }
+                            }
+                        });
+                    } catch (ParseException ex) {
+                        Exceptions.printStackTrace(ex);
                     }
                 }, NbBundle.getMessage(JavaCodeTemplateProcessor.class, "JCT-init"), cancel, false); //NOI18N
             }
         }
     }
+    
+    private boolean isPartOfWhileLoop(JTextComponent component, CompilationController controller) {
+        TreeUtilities treeUtilities = controller.getTreeUtilities();
+        TreePath currentPath = treeUtilities.pathFor(component.getCaretPosition());
+        TreePath parentPath = treeUtilities.getPathElementOfKind(Tree.Kind.WHILE_LOOP, currentPath);
+        return parentPath != null;
+    }
+    
+    private boolean isRightParenthesisOfLoopPresent(JTextComponent component, CompilationController controller) {
+        AtomicBoolean result = new AtomicBoolean(true);
+        Document document = component.getDocument();
+        document.render(() -> {
+            TokenHierarchy<?> tokenHierarchy = controller.getTokenHierarchy();
+            TokenSequence<?> tokenSequence = tokenHierarchy.tokenSequence();
+            tokenSequence.move(component.getCaretPosition());

Review comment:
       This code runs outside of the AWT event dispatch thread, so calling getCaretPosition() seems somewhat dangerous. There are various offsets available inside the task, is there some that could be used. For example "so" looks like a potential candidate?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists