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



##########
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:
       As far as I can tell from the line `String text = 
controller.getText().substring(startPos, so);` the wrong value of the end index 
of the substring is used, namely the start offset of the abbreviation, although 
(the end offset of the abbreviation + the number of spaces before the closing 
bracket, if any + 1) should be used to check for the presence of the right 
parenthesis (the problem is that the end offset of the abbreviation is not 
calculated before, at least in the  JavaCodeTemplateFilter class). Because of 
this, the `treeKindCtx` variable is always set to `null` and the code template 
for the for loop is rejected. 
   
   In addition, the code for the `WHILE_LOOP` case is never executed, because 
when the caret position is in parentheses that are part of a while loop, the 
leaf node for the tree path on which the caret is located is 
`ParenthesizedTree`, not `WhileLoopTree`. Because of this, the template for a 
while loop works even when there is no right parenthesis.




----------------------------------------------------------------
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:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

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

Reply via email to