You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by "NReib (via GitHub)" <gi...@apache.org> on 2025/03/02 03:28:03 UTC

[PR] use fully qualified name when extracting variable type [netbeans]

NReib opened a new pull request, #8287:
URL: https://github.com/apache/netbeans/pull/8287

   #8286 8286
   
   This is my first push using git.
   
   Sadly I was not able to create unit tests for this.


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

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

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

Re: [PR] use fully qualified name when extracting variable type [netbeans]

Posted by "junichi11 (via GitHub)" <gi...@apache.org>.
junichi11 commented on code in PR #8287:
URL: https://github.com/apache/netbeans/pull/8287#discussion_r1980352382


##########
php/php.editor/test/unit/src/org/netbeans/modules/php/editor/csl/OccurrencesFinderImplGH8286Test.java:
##########
@@ -0,0 +1,30 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.php.editor.csl;
+
+public class OccurrencesFinderImplGH8286Test extends OccurrencesFinderImplTestBase {
+
+    public OccurrencesFinderImplGH8286Test(String testName) {
+        super(testName);
+    }
+
+    public void testIssueGH8286_01a() throws Exception {
+        checkOccurrences(getTestPath(), "TestA::X->ge^t();", true);

Review Comment:
   Where is a result file? (***.occurrences)



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

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

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

Re: [PR] use fully qualified name when extracting variable type [netbeans]

Posted by "junichi11 (via GitHub)" <gi...@apache.org>.
junichi11 commented on PR #8287:
URL: https://github.com/apache/netbeans/pull/8287#issuecomment-2705362503

   Please also change the title of this PR. 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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

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

Re: [PR] use fully qualified name when extracting variable type [netbeans]

Posted by "NReib (via GitHub)" <gi...@apache.org>.
NReib commented on PR #8287:
URL: https://github.com/apache/netbeans/pull/8287#issuecomment-2699108181

   @junichi11 Thank you for your feedback. It seems I misunderstood something about the unit tests. I added 2 test cases and changed the implementation due to failing tests. I hope it is more clear now what is happening.
   It appears I have reformatted the code of VariousUtils, is there any Formatting Guideline active for NB?


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

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

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

Re: [PR] [GH-8286] Fix Go to Declaration for an enum method using an FQ name [netbeans]

Posted by "junichi11 (via GitHub)" <gi...@apache.org>.
junichi11 merged PR #8287:
URL: https://github.com/apache/netbeans/pull/8287


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

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

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

Re: [PR] use fully qualified name when extracting variable type [netbeans]

Posted by "junichi11 (via GitHub)" <gi...@apache.org>.
junichi11 commented on code in PR #8287:
URL: https://github.com/apache/netbeans/pull/8287#discussion_r1982382936


##########
php/php.editor/test/unit/data/testfiles/gotodeclaration/testIssueGH8286/testIssueGH8286.php:
##########
@@ -0,0 +1,45 @@
+<?php
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+namespace test;
+
+enum TestA{
+
+  case X;

Review Comment:
   Nitpick: Please use 4 spaces.



##########
php/php.editor/test/unit/data/testfiles/markoccurences/testIssueGH8286/testIssueGH8286.php.testIssueGH8286_01.occurrences:
##########
@@ -0,0 +1,2 @@
+  public function |>MARK_OCCURRENCES:get<|(string $param): string{

Review Comment:
   Nitpick: Please use 4 spaces.



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

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

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

Re: [PR] use fully qualified name when extracting variable type [netbeans]

Posted by "junichi11 (via GitHub)" <gi...@apache.org>.
junichi11 commented on code in PR #8287:
URL: https://github.com/apache/netbeans/pull/8287#discussion_r1980350114


##########
php/php.editor/src/org/netbeans/modules/php/editor/model/impl/VariousUtils.java:
##########


Review Comment:
   Don't use TAB spaces.



##########
php/php.editor/test/unit/data/testfiles/gotodeclaration/testIssueGH8286/testIssueGH8286.php:
##########
@@ -0,0 +1,42 @@
+<?php
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+
+namespace test;
+enum TestA{
+
+	case X;
+
+	public function get(string $param): string{
+		return '';
+	}
+}
+
+namespace test2;
+enum TestB{
+
+	case X;
+
+	public function get(): string{
+		return '';
+	}
+}
+
+\test\TestA::X->get();

Review Comment:
   Please use whitespaces. Add a new line.



##########
php/php.editor/test/unit/data/testfiles/gotodeclaration/testIssue270422/testIssue270422.php:
##########
@@ -8,4 +8,4 @@ class ChildClass {
 }
 
 class ParentClass {
-}
+}

Review Comment:
   Why?



##########
php/php.editor/test/unit/src/org/netbeans/modules/php/editor/csl/OccurrencesFinderImplGH8286Test.java:
##########
@@ -0,0 +1,30 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.php.editor.csl;
+
+public class OccurrencesFinderImplGH8286Test extends OccurrencesFinderImplTestBase {
+
+    public OccurrencesFinderImplGH8286Test(String testName) {
+        super(testName);
+    }
+
+    public void testIssueGH8286_01a() throws Exception {
+        checkOccurrences(getTestPath(), "TestA::X->ge^t();", true);

Review Comment:
   Where is a result file?



##########
php/php.editor/test/unit/data/testfiles/markoccurences/testIssueGH8286/testIssueGH8286.php:
##########
@@ -0,0 +1,42 @@
+<?php
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+
+namespace test;
+enum TestA{
+
+	case X;
+
+	public function get(string $param): string{
+		return '';
+	}
+}
+
+namespace test2;
+enum TestB{
+
+	case X;
+
+	public function get(): string{
+		return '';
+	}
+}
+
+\test\TestA::X->get();

Review Comment:
   Please use whitespaces. Add a new line.



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

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

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

Re: [PR] use fully qualified name when extracting variable type [netbeans]

Posted by "junichi11 (via GitHub)" <gi...@apache.org>.
junichi11 commented on PR #8287:
URL: https://github.com/apache/netbeans/pull/8287#issuecomment-2699238746

   You have to squash as one commit.
   As I wrote above, we should add an example to the commit message.
   ~~~
   [GH-8286] Fix Go to Declaration for an enum method using an FQ name
   
   Example:
   ```php
   namespace test1;
   
   enum TestA {
   
       case X;
   
       public function get(string $param): string {
           return '';
       }
   }
   
   namespace test2;
   
   \test1\TestA::X->get('xxx'); // don't work go to declaration & mark occurrences for the get method
   ``` 
   ~~~


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

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

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

Re: [PR] [GH-8286] Fix Go to Declaration for an enum method using an FQ name [netbeans]

Posted by "tmysik (via GitHub)" <gi...@apache.org>.
tmysik commented on PR #8287:
URL: https://github.com/apache/netbeans/pull/8287#issuecomment-2708242016

   (I changed the PR title now.)
   


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

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

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

Re: [PR] use fully qualified name when extracting variable type [netbeans]

Posted by "NReib (via GitHub)" <gi...@apache.org>.
NReib commented on PR #8287:
URL: https://github.com/apache/netbeans/pull/8287#issuecomment-2703160373

   @junichi11 changed to 4 spaces. Is there any correct way to push these kind of changes while keeping it in the same commit? Now I have been using --force, although I have heard it is not good to do that


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

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

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

Re: [PR] use fully qualified name when extracting variable type [netbeans]

Posted by "NReib (via GitHub)" <gi...@apache.org>.
NReib commented on PR #8287:
URL: https://github.com/apache/netbeans/pull/8287#issuecomment-2702203719

   @junichi11 squashed, changed formatting, added result files


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

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

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

Re: [PR] Fix Go to Declaration for an enum method using an FQ name [netbeans]

Posted by "tmysik (via GitHub)" <gi...@apache.org>.
tmysik commented on PR #8287:
URL: https://github.com/apache/netbeans/pull/8287#issuecomment-2708240015

   @NReib, thanks for this PR! :tada: 
   
   Regarding Git force-pushing - typically, it is totally OK, if you are the only person working on the branch. It should/cannot be used when more people work on it, since it basically rewrites the Git history (so, if user A force-pushes something, it could overwrite - and therefore loose - changes done by someone else).
   


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

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

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

Re: [PR] Fix Go to Declaration for an enum method using an FQ name [netbeans]

Posted by "tmysik (via GitHub)" <gi...@apache.org>.
tmysik commented on PR #8287:
URL: https://github.com/apache/netbeans/pull/8287#issuecomment-2708241222

   @NReib, as @junichi11 wrote, please always try to unify the issue and the PR so it is clear that they belong to each other. For the issue, I can see this:
   
   ![image](https://github.com/user-attachments/assets/acc20bdb-823b-44ab-a54f-0acff548a48c)
   
   Also, providing a code example in the PR description helps _a lot_.
   
   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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

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

Re: [PR] use fully qualified name when extracting variable type [netbeans]

Posted by "junichi11 (via GitHub)" <gi...@apache.org>.
junichi11 commented on PR #8287:
URL: https://github.com/apache/netbeans/pull/8287#issuecomment-2699164827

   > It appears I have reformatted the code of VariousUtils, is there any Formatting Guideline active for NB?
   
   Basically just follow the existing source code. Please reformat only your changes.


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

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

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

Re: [PR] use fully qualified name when extracting variable type [netbeans]

Posted by "junichi11 (via GitHub)" <gi...@apache.org>.
junichi11 commented on PR #8287:
URL: https://github.com/apache/netbeans/pull/8287#issuecomment-2696080498

   > use fully qualified name when extracting variable type
   
   It's not easy to understand what is fixed. So, I would write the following to the commit message title instead.
   
   `"[GH-8286] Fix Go to Declaration for an enum method using an FQ name"`
   
   > Sadly I was not able to create unit tests for this.
   
   It's possible. Please add unit tests for GotoDeclaration and MarkOccurrences.
   
   Please add the issue number(e.g. `[GH-8286]`) or URL to the commit message. 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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

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

Re: [PR] use fully qualified name when extracting variable type [netbeans]

Posted by "junichi11 (via GitHub)" <gi...@apache.org>.
junichi11 commented on PR #8287:
URL: https://github.com/apache/netbeans/pull/8287#issuecomment-2705252333

   This is no problem because it is a topic branch and we can see the diff(https://github.com/apache/netbeans/compare/88a3a985ba388732459deb98aa83c406755ead1b..65cb2cf61af76acf8b1e587186e1562a75d8d3f6).
   
   
   - fix something
   - `git commit --amend`
   - force push
   
   or
   
   - fix something
   - `git commit`
   - fix something
   - `git commit`
   - ...
   - review is done
   - squash them as one commit
   - force push


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

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

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