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/07/07 05:30:16 UTC

[GitHub] [netbeans] junichi11 opened a new pull request #2241: [NETBEANS-4503] Code completion does not work in functions declared inside methods

junichi11 opened a new pull request #2241:
URL: https://github.com/apache/netbeans/pull/2241


   https://issues.apache.org/jira/browse/NETBEANS-4503
   
   #### 1st commit
   
   Nested functions are in a Method/FunctionScope.
   However, they are added as elements of a NamespaceScope.
   i.e. Method/FunctionScope doesn't have nested FunctionScopes. NamespaceScope has them.
   So, (not return the child result simply,) check whether the child result contains the parent result in the `VariableScopeFinder.findWrapper()`.
   
   #### 2nd commit
   
   Unit test(testNb4503_01()) failed on GitHub Actions after I pushed the first commit.
   However, It passed on my local pc.
   
   After I've tried some things, I've somehow reproduced it.
   ```java
       public void testNb4503_01() throws Exception {
           Thread.sleep(1000); // added this, then reproduced
           testNb4503("            $e->^test(); // test1");
       }
   ```
   
   The cause:
   A cached `Source` instance may be removed after elements are indexed.
   So, the test file is scanned again when the test is run. If there are elements for LazyBuild, we may not be able to get the correct result.
   To avoiding that, after elements are scanned lazily, find the scope again.


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


[GitHub] [netbeans] junichi11 commented on a change in pull request #2241: [NETBEANS-4503] Code completion does not work in functions declared inside methods

Posted by GitBox <gi...@apache.org>.
junichi11 commented on a change in pull request #2241:
URL: https://github.com/apache/netbeans/pull/2241#discussion_r450685461



##########
File path: php/php.editor/src/org/netbeans/modules/php/editor/model/VariableScopeFinder.java
##########
@@ -45,10 +51,22 @@ public VariableScope find(final Scope scope, final int offset, final ScopeRangeA
     }
 
     public VariableScope find(final List<? extends ModelElement> elements, final int offset, final ScopeRangeAcceptor scopeRangeAcceptor) {
-        return findWrapper(elements, offset, scopeRangeAcceptor).getVariableScope();
+        AtomicBoolean isLazilyScanned = new AtomicBoolean(false);
+        VariableScope variableScope = findWrapper(elements, offset, scopeRangeAcceptor, isLazilyScanned).getVariableScope();
+        if (isLazilyScanned.get()) {
+            // some scopes may be added new elements when LazyBuild elements are scanned.
+            // so, find again.
+            // e.g. Source instances are cached as weak references.
+            // so, ParserResult may not be the same instance even if the FileObject is the same.
+            // it means that new Model and new ModelVisitor are created again. in such case, LazyBuild elements may not be scanned yet.
+            LOGGER.log(Level.FINE, "(LazyBuild)Scope is scanned."); // NOI18N
+            isLazilyScanned.set(false);

Review comment:
       yes, will remove.




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


[GitHub] [netbeans] tmysik commented on a change in pull request #2241: [NETBEANS-4503] Code completion does not work in functions declared inside methods

Posted by GitBox <gi...@apache.org>.
tmysik commented on a change in pull request #2241:
URL: https://github.com/apache/netbeans/pull/2241#discussion_r452195146



##########
File path: php/php.editor/src/org/netbeans/modules/php/editor/model/VariableScopeFinder.java
##########
@@ -45,10 +51,22 @@ public VariableScope find(final Scope scope, final int offset, final ScopeRangeA
     }
 
     public VariableScope find(final List<? extends ModelElement> elements, final int offset, final ScopeRangeAcceptor scopeRangeAcceptor) {
-        return findWrapper(elements, offset, scopeRangeAcceptor).getVariableScope();
+        AtomicBoolean isLazilyScanned = new AtomicBoolean(false);
+        VariableScope variableScope = findWrapper(elements, offset, scopeRangeAcceptor, isLazilyScanned).getVariableScope();
+        if (isLazilyScanned.get()) {
+            // some scopes may be added new elements when LazyBuild elements are scanned.
+            // so, find again.
+            // e.g. Source instances are cached as weak references.
+            // so, ParserResult may not be the same instance even if the FileObject is the same.
+            // it means that new Model and new ModelVisitor are created again. in such case, LazyBuild elements may not be scanned yet.
+            LOGGER.log(Level.FINE, "(LazyBuild)Scope is scanned."); // NOI18N
+            isLazilyScanned.set(false);

Review comment:
       No problem, my fault because of my misleading/confusing comment.
   




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


[GitHub] [netbeans] tmysik commented on pull request #2241: [NETBEANS-4503] Code completion does not work in functions declared inside methods

Posted by GitBox <gi...@apache.org>.
tmysik commented on pull request #2241:
URL: https://github.com/apache/netbeans/pull/2241#issuecomment-654751269


   @junichi11 
   
   Looks good to me, will merge once CI passes.
   


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


[GitHub] [netbeans] junichi11 commented on pull request #2241: [NETBEANS-4503] Code completion does not work in functions declared inside methods

Posted by GitBox <gi...@apache.org>.
junichi11 commented on pull request #2241:
URL: https://github.com/apache/netbeans/pull/2241#issuecomment-654740344


   @tmysik Fixed https://github.com/apache/netbeans/compare/117cdec493f2a6685c94c9635eb8b54e5f1d8f0b..9fa8489abf12899ad57e22bf1d586d645c25f3cb.
   Maybe, CI takes about 2 hours. Thank you!


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


[GitHub] [netbeans] tmysik commented on pull request #2241: [NETBEANS-4503] Code completion does not work in functions declared inside methods

Posted by GitBox <gi...@apache.org>.
tmysik commented on pull request #2241:
URL: https://github.com/apache/netbeans/pull/2241#issuecomment-654669674


   @junichi11 Let me know, I am ready to merge it as it is now. Thanks!
   


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


[GitHub] [netbeans] tmysik commented on a change in pull request #2241: [NETBEANS-4503] Code completion does not work in functions declared inside methods

Posted by GitBox <gi...@apache.org>.
tmysik commented on a change in pull request #2241:
URL: https://github.com/apache/netbeans/pull/2241#discussion_r450675149



##########
File path: php/php.editor/src/org/netbeans/modules/php/editor/model/VariableScopeFinder.java
##########
@@ -45,10 +51,22 @@ public VariableScope find(final Scope scope, final int offset, final ScopeRangeA
     }
 
     public VariableScope find(final List<? extends ModelElement> elements, final int offset, final ScopeRangeAcceptor scopeRangeAcceptor) {
-        return findWrapper(elements, offset, scopeRangeAcceptor).getVariableScope();
+        AtomicBoolean isLazilyScanned = new AtomicBoolean(false);
+        VariableScope variableScope = findWrapper(elements, offset, scopeRangeAcceptor, isLazilyScanned).getVariableScope();
+        if (isLazilyScanned.get()) {
+            // some scopes may be added new elements when LazyBuild elements are scanned.
+            // so, find again.
+            // e.g. Source instances are cached as weak references.
+            // so, ParserResult may not be the same instance even if the FileObject is the same.
+            // it means that new Model and new ModelVisitor are created again. in such case, LazyBuild elements may not be scanned yet.
+            LOGGER.log(Level.FINE, "(LazyBuild)Scope is scanned."); // NOI18N
+            isLazilyScanned.set(false);

Review comment:
       Ha, it is running in just one thread, right? In such a case, not needed, of course.
   

##########
File path: php/php.editor/src/org/netbeans/modules/php/editor/model/VariableScopeFinder.java
##########
@@ -45,10 +51,22 @@ public VariableScope find(final Scope scope, final int offset, final ScopeRangeA
     }
 
     public VariableScope find(final List<? extends ModelElement> elements, final int offset, final ScopeRangeAcceptor scopeRangeAcceptor) {
-        return findWrapper(elements, offset, scopeRangeAcceptor).getVariableScope();
+        AtomicBoolean isLazilyScanned = new AtomicBoolean(false);
+        VariableScope variableScope = findWrapper(elements, offset, scopeRangeAcceptor, isLazilyScanned).getVariableScope();
+        if (isLazilyScanned.get()) {
+            // some scopes may be added new elements when LazyBuild elements are scanned.
+            // so, find again.
+            // e.g. Source instances are cached as weak references.
+            // so, ParserResult may not be the same instance even if the FileObject is the same.
+            // it means that new Model and new ModelVisitor are created again. in such case, LazyBuild elements may not be scanned yet.
+            LOGGER.log(Level.FINE, "(LazyBuild)Scope is scanned."); // NOI18N
+            isLazilyScanned.set(false);

Review comment:
       Maybe better [`compareAndSet()`](https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/atomic/AtomicBoolean.html#compareAndSet-boolean-boolean-)?
   

##########
File path: php/php.editor/src/org/netbeans/modules/php/editor/model/VariableScopeFinder.java
##########
@@ -45,10 +51,22 @@ public VariableScope find(final Scope scope, final int offset, final ScopeRangeA
     }
 
     public VariableScope find(final List<? extends ModelElement> elements, final int offset, final ScopeRangeAcceptor scopeRangeAcceptor) {
-        return findWrapper(elements, offset, scopeRangeAcceptor).getVariableScope();
+        AtomicBoolean isLazilyScanned = new AtomicBoolean(false);
+        VariableScope variableScope = findWrapper(elements, offset, scopeRangeAcceptor, isLazilyScanned).getVariableScope();
+        if (isLazilyScanned.get()) {
+            // some scopes may be added new elements when LazyBuild elements are scanned.
+            // so, find again.
+            // e.g. Source instances are cached as weak references.
+            // so, ParserResult may not be the same instance even if the FileObject is the same.
+            // it means that new Model and new ModelVisitor are created again. in such case, LazyBuild elements may not be scanned yet.
+            LOGGER.log(Level.FINE, "(LazyBuild)Scope is scanned."); // NOI18N
+            isLazilyScanned.set(false);
+            variableScope = findWrapper(elements, offset, scopeRangeAcceptor, isLazilyScanned).getVariableScope();
+        }
+        return variableScope;
     }
 
-    private VariableScopeWrapper findWrapper(final List<? extends ModelElement> elements, final int offset, final ScopeRangeAcceptor scopeRangeAcceptor) {
+    private VariableScopeWrapper findWrapper(final List<? extends ModelElement> elements, final int offset, final ScopeRangeAcceptor scopeRangeAcceptor, final AtomicBoolean isLazilyScanned) {

Review comment:
       Missing `@Nullable`?
   




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


[GitHub] [netbeans] junichi11 commented on a change in pull request #2241: [NETBEANS-4503] Code completion does not work in functions declared inside methods

Posted by GitBox <gi...@apache.org>.
junichi11 commented on a change in pull request #2241:
URL: https://github.com/apache/netbeans/pull/2241#discussion_r450686966



##########
File path: php/php.editor/src/org/netbeans/modules/php/editor/model/VariableScopeFinder.java
##########
@@ -45,10 +51,22 @@ public VariableScope find(final Scope scope, final int offset, final ScopeRangeA
     }
 
     public VariableScope find(final List<? extends ModelElement> elements, final int offset, final ScopeRangeAcceptor scopeRangeAcceptor) {
-        return findWrapper(elements, offset, scopeRangeAcceptor).getVariableScope();
+        AtomicBoolean isLazilyScanned = new AtomicBoolean(false);
+        VariableScope variableScope = findWrapper(elements, offset, scopeRangeAcceptor, isLazilyScanned).getVariableScope();
+        if (isLazilyScanned.get()) {
+            // some scopes may be added new elements when LazyBuild elements are scanned.
+            // so, find again.
+            // e.g. Source instances are cached as weak references.
+            // so, ParserResult may not be the same instance even if the FileObject is the same.
+            // it means that new Model and new ModelVisitor are created again. in such case, LazyBuild elements may not be scanned yet.
+            LOGGER.log(Level.FINE, "(LazyBuild)Scope is scanned."); // NOI18N
+            isLazilyScanned.set(false);
+            variableScope = findWrapper(elements, offset, scopeRangeAcceptor, isLazilyScanned).getVariableScope();
+        }
+        return variableScope;
     }
 
-    private VariableScopeWrapper findWrapper(final List<? extends ModelElement> elements, final int offset, final ScopeRangeAcceptor scopeRangeAcceptor) {
+    private VariableScopeWrapper findWrapper(final List<? extends ModelElement> elements, final int offset, final ScopeRangeAcceptor scopeRangeAcceptor, final AtomicBoolean isLazilyScanned) {

Review comment:
       Will add. Thanks!




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


[GitHub] [netbeans] junichi11 commented on a change in pull request #2241: [NETBEANS-4503] Code completion does not work in functions declared inside methods

Posted by GitBox <gi...@apache.org>.
junichi11 commented on a change in pull request #2241:
URL: https://github.com/apache/netbeans/pull/2241#discussion_r451208538



##########
File path: php/php.editor/src/org/netbeans/modules/php/editor/model/VariableScopeFinder.java
##########
@@ -45,10 +51,22 @@ public VariableScope find(final Scope scope, final int offset, final ScopeRangeA
     }
 
     public VariableScope find(final List<? extends ModelElement> elements, final int offset, final ScopeRangeAcceptor scopeRangeAcceptor) {
-        return findWrapper(elements, offset, scopeRangeAcceptor).getVariableScope();
+        AtomicBoolean isLazilyScanned = new AtomicBoolean(false);
+        VariableScope variableScope = findWrapper(elements, offset, scopeRangeAcceptor, isLazilyScanned).getVariableScope();
+        if (isLazilyScanned.get()) {
+            // some scopes may be added new elements when LazyBuild elements are scanned.
+            // so, find again.
+            // e.g. Source instances are cached as weak references.
+            // so, ParserResult may not be the same instance even if the FileObject is the same.
+            // it means that new Model and new ModelVisitor are created again. in such case, LazyBuild elements may not be scanned yet.
+            LOGGER.log(Level.FINE, "(LazyBuild)Scope is scanned."); // NOI18N
+            isLazilyScanned.set(false);

Review comment:
       I seem to misunderstand something. Sorry... I'll revert it if I modify this class in the future. Thank you for your review :)




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


[GitHub] [netbeans] tmysik commented on a change in pull request #2241: [NETBEANS-4503] Code completion does not work in functions declared inside methods

Posted by GitBox <gi...@apache.org>.
tmysik commented on a change in pull request #2241:
URL: https://github.com/apache/netbeans/pull/2241#discussion_r450757749



##########
File path: php/php.editor/src/org/netbeans/modules/php/editor/model/VariableScopeFinder.java
##########
@@ -45,10 +51,22 @@ public VariableScope find(final Scope scope, final int offset, final ScopeRangeA
     }
 
     public VariableScope find(final List<? extends ModelElement> elements, final int offset, final ScopeRangeAcceptor scopeRangeAcceptor) {
-        return findWrapper(elements, offset, scopeRangeAcceptor).getVariableScope();
+        AtomicBoolean isLazilyScanned = new AtomicBoolean(false);
+        VariableScope variableScope = findWrapper(elements, offset, scopeRangeAcceptor, isLazilyScanned).getVariableScope();
+        if (isLazilyScanned.get()) {
+            // some scopes may be added new elements when LazyBuild elements are scanned.
+            // so, find again.
+            // e.g. Source instances are cached as weak references.
+            // so, ParserResult may not be the same instance even if the FileObject is the same.
+            // it means that new Model and new ModelVisitor are created again. in such case, LazyBuild elements may not be scanned yet.
+            LOGGER.log(Level.FINE, "(LazyBuild)Scope is scanned."); // NOI18N
+            isLazilyScanned.set(false);

Review comment:
       If it is running in only one thread (as here I guess, in the _parsing_ thread) then `AtomicBoolean` is just a holder for a boolean. So, using `compareAndSet()` is IMHO not necessary.
   




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


[GitHub] [netbeans] junichi11 commented on a change in pull request #2241: [NETBEANS-4503] Code completion does not work in functions declared inside methods

Posted by GitBox <gi...@apache.org>.
junichi11 commented on a change in pull request #2241:
URL: https://github.com/apache/netbeans/pull/2241#discussion_r450739205



##########
File path: php/php.editor/src/org/netbeans/modules/php/editor/model/VariableScopeFinder.java
##########
@@ -45,10 +51,22 @@ public VariableScope find(final Scope scope, final int offset, final ScopeRangeA
     }
 
     public VariableScope find(final List<? extends ModelElement> elements, final int offset, final ScopeRangeAcceptor scopeRangeAcceptor) {
-        return findWrapper(elements, offset, scopeRangeAcceptor).getVariableScope();
+        AtomicBoolean isLazilyScanned = new AtomicBoolean(false);
+        VariableScope variableScope = findWrapper(elements, offset, scopeRangeAcceptor, isLazilyScanned).getVariableScope();
+        if (isLazilyScanned.get()) {
+            // some scopes may be added new elements when LazyBuild elements are scanned.
+            // so, find again.
+            // e.g. Source instances are cached as weak references.
+            // so, ParserResult may not be the same instance even if the FileObject is the same.
+            // it means that new Model and new ModelVisitor are created again. in such case, LazyBuild elements may not be scanned yet.
+            LOGGER.log(Level.FINE, "(LazyBuild)Scope is scanned."); // NOI18N
+            isLazilyScanned.set(false);

Review comment:
       Oops, will change it to your suggestion :)




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


[GitHub] [netbeans] tmysik merged pull request #2241: [NETBEANS-4503] Code completion does not work in functions declared inside methods

Posted by GitBox <gi...@apache.org>.
tmysik merged pull request #2241:
URL: https://github.com/apache/netbeans/pull/2241


   


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