You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by "troizet (via GitHub)" <gi...@apache.org> on 2023/03/10 15:01:08 UTC

[GitHub] [netbeans] troizet opened a new pull request, #5648: PHP: Adding a new hint ReturnStatementWithoutValueHintError.

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

   Adding a new hint that checks if the return statement has a value if the return type is specified for the function, as suggested in #5078.
   


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


[GitHub] [netbeans] troizet commented on a diff in pull request #5648: PHP: Adding a new hint ReturnStatementWithoutValueHintError.

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


##########
php/php.editor/src/org/netbeans/modules/php/editor/verification/ReturnTypeHintError.java:
##########
@@ -133,17 +140,17 @@ private void checkReturnType(Map<ASTNode, Set<ReturnStatement>> returnStatements
         "ReturnTypeHintErrorVoidDesc=\"{0}\" cannot return anything"
     })
     private void checkVoidAndNeverReturnStatements(Set<ReturnStatement> statements, String name, List<Hint> hints) {
-        if (Type.VOID.equals(name) || isNeverType(name)) {
+        if (isVoidType(name) || isNeverType(name)) {
             // check empty return statement
-            statements.forEach((statement) -> {
+            for (ReturnStatement statement: statements) {

Review Comment:
   Yes, you are right.



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


[GitHub] [netbeans] troizet commented on a diff in pull request #5648: PHP: Adding a new hint ReturnStatementWithoutValueHintError.

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


##########
php/php.editor/src/org/netbeans/modules/php/editor/verification/ReturnTypeHintError.java:
##########
@@ -116,13 +118,18 @@ private void checkReturnType(Map<ASTNode, Set<ReturnStatement>> returnStatements
                 NamespaceName namespaceName = (NamespaceName) returnType;
                 String name = CodeUtils.extractUnqualifiedName(namespaceName);
                 checkVoidAndNeverReturnStatements(statements, name, hints);
+                checkReturnStatementsWithoutValue(statements, name, hints);
             } else if (returnType instanceof NullableType) {
                 Expression type = ((NullableType) returnType).getType();
                 if (type instanceof NamespaceName) {
                     NamespaceName namespaceName = (NamespaceName) type;
                     String name = CodeUtils.extractUnqualifiedName(namespaceName);
                     checkInvalidVoidAndNeverReturnType(type, name, hints);
-                }
+                } 

Review Comment:
   Done.



##########
php/php.editor/src/org/netbeans/modules/php/editor/verification/ReturnTypeHintError.java:
##########
@@ -152,17 +159,40 @@ private void checkVoidAndNeverReturnStatements(Set<ReturnStatement> statements,
         "ReturnTypeHintErrorInvalidVoidDesc=\"{0}\" cannot be used with \"?\""
     })
     private void checkInvalidVoidAndNeverReturnType(Expression returnType, String name, List<Hint> hints) {
-        if (Type.VOID.equals(name)
+        if (isVoidType(name)
                 || isNeverType(name)) {
             addHint(returnType, Bundle.ReturnTypeHintErrorInvalidVoidDesc(name), hints);
         }
     }
 
+    @NbBundle.Messages({
+        "ReturnStatementWithoutValueHintErrorDesc=Return statement must be filled with the value"
+    })
+    private void checkReturnStatementsWithoutValue(Set<ReturnStatement> statements, String name, List<Hint> hints) {
+        if (!isVoidType(name) && !isNeverType(name)) {
+            // check empty return statement
+            for (ReturnStatement statement: statements) {
+                if (CancelSupport.getDefault().isCancelled()) {
+                    return;
+                }
+                Expression expression = statement.getExpression();
+                if (expression == null) {
+                    addHint(statement, Bundle.ReturnStatementWithoutValueHintErrorDesc(), hints);
+                }
+            }
+        }
+    }
+
     private boolean isNeverType(String name) {
         return getPhpVersion(fileObject).hasNeverType()
                 && Type.NEVER.equals(name);
     }
 
+   private boolean isVoidType(String name) {
+        return getPhpVersion(fileObject).hasVoidReturnType()
+                && Type.VOID.equals(name);

Review Comment:
   Done.



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


[GitHub] [netbeans] troizet commented on a diff in pull request #5648: PHP: Adding a new hint ReturnStatementWithoutValueHintError.

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


##########
php/php.editor/src/org/netbeans/modules/php/editor/verification/ReturnStatementWithoutValueHintError.java:
##########
@@ -0,0 +1,218 @@
+/*
+ * 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.verification;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Set;
+import org.netbeans.api.annotations.common.NullAllowed;
+import org.netbeans.modules.csl.api.Hint;
+import org.netbeans.modules.csl.api.HintFix;
+import org.netbeans.modules.csl.api.OffsetRange;
+import org.netbeans.modules.csl.spi.support.CancelSupport;
+import org.netbeans.modules.php.api.PhpVersion;
+import org.netbeans.modules.php.editor.CodeUtils;
+import org.netbeans.modules.php.editor.model.FileScope;
+import org.netbeans.modules.php.editor.model.impl.Type;
+import org.netbeans.modules.php.editor.parser.PHPParseResult;
+import org.netbeans.modules.php.editor.parser.astnodes.ASTNode;
+import org.netbeans.modules.php.editor.parser.astnodes.Expression;
+import org.netbeans.modules.php.editor.parser.astnodes.FunctionDeclaration;
+import org.netbeans.modules.php.editor.parser.astnodes.IntersectionType;
+import org.netbeans.modules.php.editor.parser.astnodes.LambdaFunctionDeclaration;
+import org.netbeans.modules.php.editor.parser.astnodes.NamespaceName;
+import org.netbeans.modules.php.editor.parser.astnodes.NullableType;
+import org.netbeans.modules.php.editor.parser.astnodes.ReturnStatement;
+import org.netbeans.modules.php.editor.parser.astnodes.UnionType;
+import org.netbeans.modules.php.editor.parser.astnodes.visitors.DefaultVisitor;
+import org.openide.filesystems.FileObject;
+import org.openide.util.NbBundle;
+
+/**
+ * Checks if the return statement has a value if a return type is specified for the function
+ *
+ */
+public class ReturnStatementWithoutValueHintError extends HintErrorRule {
+
+    private FileObject fileObject;
+
+    @NbBundle.Messages("ReturnStatementWithoutValueHintErrorName=Return statement without value")
+    @Override
+    public String getDisplayName() {
+        return Bundle.ReturnStatementWithoutValueHintErrorName();
+    }
+
+    @Override
+    public void invoke(PHPRuleContext context, List<Hint> hints) {
+        PHPParseResult phpParseResult = (PHPParseResult) context.parserResult;
+        if (phpParseResult.getProgram() == null) {
+            return;
+        }
+        FileScope fileScope = context.fileScope;
+        fileObject = phpParseResult.getSnapshot().getSource().getFileObject();
+        if (fileScope != null && fileObject != null && appliesTo(fileObject)) {
+            if (CancelSupport.getDefault().isCancelled()) {
+                return;
+            }
+            CheckVisitor checkVisitor = new CheckVisitor();
+            phpParseResult.getProgram().accept(checkVisitor);
+            Map<ASTNode, Set<ReturnStatement>> returnStatements = checkVisitor.getReturnStatements();
+            checkReturnType(returnStatements, hints);
+        }
+    }
+
+    protected PhpVersion getPhpVersion(@NullAllowed FileObject file) {
+        if (file == null) {
+            return PhpVersion.getDefault();
+        }
+        return CodeUtils.getPhpVersion(file);
+    }
+
+    private boolean appliesTo(FileObject file) {
+        return getPhpVersion(file).compareTo(PhpVersion.PHP_70) >= 0;
+    }
+
+    private void checkReturnType(Map<ASTNode, Set<ReturnStatement>> returnStatements, List<Hint> hints) {
+        for (Entry<ASTNode, Set<ReturnStatement>> entry : returnStatements.entrySet()) {
+            if (CancelSupport.getDefault().isCancelled()) {
+                return;
+            }
+            ASTNode node = entry.getKey();
+            Expression returnType = null;
+            if (node instanceof FunctionDeclaration) {
+                returnType = ((FunctionDeclaration) node).getReturnType();
+            } else if (node instanceof LambdaFunctionDeclaration) {
+                returnType = ((LambdaFunctionDeclaration) node).getReturnType();
+            }
+
+            if (returnType == null) {
+                continue;
+            }
+            Set<ReturnStatement> statements = entry.getValue();
+
+            if (returnType instanceof NamespaceName) {
+                NamespaceName namespaceName = (NamespaceName) returnType;
+                String name = CodeUtils.extractUnqualifiedName(namespaceName);
+                checkReturnStatementsWithoutValue(statements, name, hints);
+            } else if (returnType instanceof NullableType
+                    || returnType instanceof UnionType
+                    || returnType instanceof IntersectionType) {
+                checkReturnStatementsWithoutValue(statements, "", hints);
+            }
+
+        }
+    }
+
+    @NbBundle.Messages({
+        "ReturnStatementWithoutValueHintErrorDesc=Return statement must be filled with the value"
+    })
+    private void checkReturnStatementsWithoutValue(Set<ReturnStatement> statements, String name, List<Hint> hints) {
+        if (!Type.VOID.equals(name) && !isNeverType(name)) {
+            // check empty return statement
+            statements.forEach((statement) -> {
+                if (CancelSupport.getDefault().isCancelled()) {
+                    return;

Review Comment:
   Fixed.



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


[GitHub] [netbeans] tmysik commented on a diff in pull request #5648: PHP: Adding a new hint ReturnStatementWithoutValueHintError.

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


##########
php/php.editor/src/org/netbeans/modules/php/editor/verification/ReturnTypeHintError.java:
##########
@@ -133,17 +140,17 @@ private void checkReturnType(Map<ASTNode, Set<ReturnStatement>> returnStatements
         "ReturnTypeHintErrorVoidDesc=\"{0}\" cannot return anything"
     })
     private void checkVoidAndNeverReturnStatements(Set<ReturnStatement> statements, String name, List<Hint> hints) {
-        if (Type.VOID.equals(name) || isNeverType(name)) {
+        if (isVoidType(name) || isNeverType(name)) {
             // check empty return statement
-            statements.forEach((statement) -> {
+            for (ReturnStatement statement: statements) {

Review Comment:
   Why this change?
   



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


[GitHub] [netbeans] junichi11 merged pull request #5648: PHP: Adding a new error hint, as suggested in #5078.

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


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


[GitHub] [netbeans] mbien commented on a diff in pull request #5648: PHP: Adding a new hint ReturnStatementWithoutValueHintError.

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


##########
php/php.editor/src/org/netbeans/modules/php/editor/verification/ReturnStatementWithoutValueHintError.java:
##########
@@ -0,0 +1,218 @@
+/*
+ * 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.verification;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Set;
+import org.netbeans.api.annotations.common.NullAllowed;
+import org.netbeans.modules.csl.api.Hint;
+import org.netbeans.modules.csl.api.HintFix;
+import org.netbeans.modules.csl.api.OffsetRange;
+import org.netbeans.modules.csl.spi.support.CancelSupport;
+import org.netbeans.modules.php.api.PhpVersion;
+import org.netbeans.modules.php.editor.CodeUtils;
+import org.netbeans.modules.php.editor.model.FileScope;
+import org.netbeans.modules.php.editor.model.impl.Type;
+import org.netbeans.modules.php.editor.parser.PHPParseResult;
+import org.netbeans.modules.php.editor.parser.astnodes.ASTNode;
+import org.netbeans.modules.php.editor.parser.astnodes.Expression;
+import org.netbeans.modules.php.editor.parser.astnodes.FunctionDeclaration;
+import org.netbeans.modules.php.editor.parser.astnodes.IntersectionType;
+import org.netbeans.modules.php.editor.parser.astnodes.LambdaFunctionDeclaration;
+import org.netbeans.modules.php.editor.parser.astnodes.NamespaceName;
+import org.netbeans.modules.php.editor.parser.astnodes.NullableType;
+import org.netbeans.modules.php.editor.parser.astnodes.ReturnStatement;
+import org.netbeans.modules.php.editor.parser.astnodes.UnionType;
+import org.netbeans.modules.php.editor.parser.astnodes.visitors.DefaultVisitor;
+import org.openide.filesystems.FileObject;
+import org.openide.util.NbBundle;
+
+/**
+ * Checks if the return statement has a value if a return type is specified for the function
+ *
+ */
+public class ReturnStatementWithoutValueHintError extends HintErrorRule {
+
+    private FileObject fileObject;
+
+    @NbBundle.Messages("ReturnStatementWithoutValueHintErrorName=Return statement without value")
+    @Override
+    public String getDisplayName() {
+        return Bundle.ReturnStatementWithoutValueHintErrorName();
+    }
+
+    @Override
+    public void invoke(PHPRuleContext context, List<Hint> hints) {
+        PHPParseResult phpParseResult = (PHPParseResult) context.parserResult;
+        if (phpParseResult.getProgram() == null) {
+            return;
+        }
+        FileScope fileScope = context.fileScope;
+        fileObject = phpParseResult.getSnapshot().getSource().getFileObject();
+        if (fileScope != null && fileObject != null && appliesTo(fileObject)) {
+            if (CancelSupport.getDefault().isCancelled()) {
+                return;
+            }
+            CheckVisitor checkVisitor = new CheckVisitor();
+            phpParseResult.getProgram().accept(checkVisitor);
+            Map<ASTNode, Set<ReturnStatement>> returnStatements = checkVisitor.getReturnStatements();
+            checkReturnType(returnStatements, hints);
+        }
+    }
+
+    protected PhpVersion getPhpVersion(@NullAllowed FileObject file) {
+        if (file == null) {
+            return PhpVersion.getDefault();
+        }
+        return CodeUtils.getPhpVersion(file);
+    }
+
+    private boolean appliesTo(FileObject file) {
+        return getPhpVersion(file).compareTo(PhpVersion.PHP_70) >= 0;
+    }
+
+    private void checkReturnType(Map<ASTNode, Set<ReturnStatement>> returnStatements, List<Hint> hints) {
+        for (Entry<ASTNode, Set<ReturnStatement>> entry : returnStatements.entrySet()) {
+            if (CancelSupport.getDefault().isCancelled()) {
+                return;
+            }
+            ASTNode node = entry.getKey();
+            Expression returnType = null;
+            if (node instanceof FunctionDeclaration) {
+                returnType = ((FunctionDeclaration) node).getReturnType();
+            } else if (node instanceof LambdaFunctionDeclaration) {
+                returnType = ((LambdaFunctionDeclaration) node).getReturnType();
+            }
+
+            if (returnType == null) {
+                continue;
+            }
+            Set<ReturnStatement> statements = entry.getValue();
+
+            if (returnType instanceof NamespaceName) {
+                NamespaceName namespaceName = (NamespaceName) returnType;
+                String name = CodeUtils.extractUnqualifiedName(namespaceName);
+                checkReturnStatementsWithoutValue(statements, name, hints);
+            } else if (returnType instanceof NullableType
+                    || returnType instanceof UnionType
+                    || returnType instanceof IntersectionType) {
+                checkReturnStatementsWithoutValue(statements, "", hints);
+            }
+
+        }
+    }
+
+    @NbBundle.Messages({
+        "ReturnStatementWithoutValueHintErrorDesc=Return statement must be filled with the value"
+    })
+    private void checkReturnStatementsWithoutValue(Set<ReturnStatement> statements, String name, List<Hint> hints) {
+        if (!Type.VOID.equals(name) && !isNeverType(name)) {
+            // check empty return statement
+            statements.forEach((statement) -> {
+                if (CancelSupport.getDefault().isCancelled()) {
+                    return;

Review Comment:
   good catch. This looks like a little cancelability bug. Both cases should be reverted to normal loops. This is probably from the time when NB had a hint which recommended to use the for each loop - and someone did this by accident and/or blindly. I turned that hint off via #3300.
   
   If you want you can simply add another commit to this PR which fixes this and we merge this unsquashed. Might be worth checking if this happens in other hints too.



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


[GitHub] [netbeans] mbien commented on a diff in pull request #5648: PHP: Adding a new hint ReturnStatementWithoutValueHintError.

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


##########
php/php.editor/src/org/netbeans/modules/php/editor/verification/ReturnStatementWithoutValueHintError.java:
##########
@@ -0,0 +1,218 @@
+/*
+ * 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.verification;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Set;
+import org.netbeans.api.annotations.common.NullAllowed;
+import org.netbeans.modules.csl.api.Hint;
+import org.netbeans.modules.csl.api.HintFix;
+import org.netbeans.modules.csl.api.OffsetRange;
+import org.netbeans.modules.csl.spi.support.CancelSupport;
+import org.netbeans.modules.php.api.PhpVersion;
+import org.netbeans.modules.php.editor.CodeUtils;
+import org.netbeans.modules.php.editor.model.FileScope;
+import org.netbeans.modules.php.editor.model.impl.Type;
+import org.netbeans.modules.php.editor.parser.PHPParseResult;
+import org.netbeans.modules.php.editor.parser.astnodes.ASTNode;
+import org.netbeans.modules.php.editor.parser.astnodes.Expression;
+import org.netbeans.modules.php.editor.parser.astnodes.FunctionDeclaration;
+import org.netbeans.modules.php.editor.parser.astnodes.IntersectionType;
+import org.netbeans.modules.php.editor.parser.astnodes.LambdaFunctionDeclaration;
+import org.netbeans.modules.php.editor.parser.astnodes.NamespaceName;
+import org.netbeans.modules.php.editor.parser.astnodes.NullableType;
+import org.netbeans.modules.php.editor.parser.astnodes.ReturnStatement;
+import org.netbeans.modules.php.editor.parser.astnodes.UnionType;
+import org.netbeans.modules.php.editor.parser.astnodes.visitors.DefaultVisitor;
+import org.openide.filesystems.FileObject;
+import org.openide.util.NbBundle;
+
+/**
+ * Checks if the return statement has a value if a return type is specified for the function
+ *
+ */
+public class ReturnStatementWithoutValueHintError extends HintErrorRule {
+
+    private FileObject fileObject;
+
+    @NbBundle.Messages("ReturnStatementWithoutValueHintErrorName=Return statement without value")
+    @Override
+    public String getDisplayName() {
+        return Bundle.ReturnStatementWithoutValueHintErrorName();
+    }
+
+    @Override
+    public void invoke(PHPRuleContext context, List<Hint> hints) {
+        PHPParseResult phpParseResult = (PHPParseResult) context.parserResult;
+        if (phpParseResult.getProgram() == null) {
+            return;
+        }
+        FileScope fileScope = context.fileScope;
+        fileObject = phpParseResult.getSnapshot().getSource().getFileObject();
+        if (fileScope != null && fileObject != null && appliesTo(fileObject)) {
+            if (CancelSupport.getDefault().isCancelled()) {
+                return;
+            }
+            CheckVisitor checkVisitor = new CheckVisitor();
+            phpParseResult.getProgram().accept(checkVisitor);
+            Map<ASTNode, Set<ReturnStatement>> returnStatements = checkVisitor.getReturnStatements();
+            checkReturnType(returnStatements, hints);
+        }
+    }
+
+    protected PhpVersion getPhpVersion(@NullAllowed FileObject file) {
+        if (file == null) {
+            return PhpVersion.getDefault();
+        }
+        return CodeUtils.getPhpVersion(file);
+    }
+
+    private boolean appliesTo(FileObject file) {
+        return getPhpVersion(file).compareTo(PhpVersion.PHP_70) >= 0;
+    }
+
+    private void checkReturnType(Map<ASTNode, Set<ReturnStatement>> returnStatements, List<Hint> hints) {
+        for (Entry<ASTNode, Set<ReturnStatement>> entry : returnStatements.entrySet()) {
+            if (CancelSupport.getDefault().isCancelled()) {
+                return;
+            }
+            ASTNode node = entry.getKey();
+            Expression returnType = null;
+            if (node instanceof FunctionDeclaration) {
+                returnType = ((FunctionDeclaration) node).getReturnType();
+            } else if (node instanceof LambdaFunctionDeclaration) {
+                returnType = ((LambdaFunctionDeclaration) node).getReturnType();
+            }
+
+            if (returnType == null) {
+                continue;
+            }
+            Set<ReturnStatement> statements = entry.getValue();
+
+            if (returnType instanceof NamespaceName) {
+                NamespaceName namespaceName = (NamespaceName) returnType;
+                String name = CodeUtils.extractUnqualifiedName(namespaceName);
+                checkReturnStatementsWithoutValue(statements, name, hints);
+            } else if (returnType instanceof NullableType
+                    || returnType instanceof UnionType
+                    || returnType instanceof IntersectionType) {
+                checkReturnStatementsWithoutValue(statements, "", hints);
+            }
+
+        }
+    }
+
+    @NbBundle.Messages({
+        "ReturnStatementWithoutValueHintErrorDesc=Return statement must be filled with the value"
+    })
+    private void checkReturnStatementsWithoutValue(Set<ReturnStatement> statements, String name, List<Hint> hints) {
+        if (!Type.VOID.equals(name) && !isNeverType(name)) {
+            // check empty return statement
+            statements.forEach((statement) -> {
+                if (CancelSupport.getDefault().isCancelled()) {
+                    return;

Review Comment:
   on second thought, this is so minor that you can simply add this to your first commit. feel free to force push if you find other 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


[GitHub] [netbeans] tmysik commented on a diff in pull request #5648: PHP: Adding a new hint ReturnStatementWithoutValueHintError.

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


##########
php/php.editor/src/org/netbeans/modules/php/editor/verification/ReturnTypeHintError.java:
##########
@@ -152,17 +159,40 @@ private void checkVoidAndNeverReturnStatements(Set<ReturnStatement> statements,
         "ReturnTypeHintErrorInvalidVoidDesc=\"{0}\" cannot be used with \"?\""
     })
     private void checkInvalidVoidAndNeverReturnType(Expression returnType, String name, List<Hint> hints) {
-        if (Type.VOID.equals(name)
+        if (isVoidType(name)
                 || isNeverType(name)) {
             addHint(returnType, Bundle.ReturnTypeHintErrorInvalidVoidDesc(name), hints);
         }
     }
 
+    @NbBundle.Messages({
+        "ReturnStatementWithoutValueHintErrorDesc=Return statement must be filled with the value"
+    })
+    private void checkReturnStatementsWithoutValue(Set<ReturnStatement> statements, String name, List<Hint> hints) {
+        if (!isVoidType(name) && !isNeverType(name)) {
+            // check empty return statement
+            for (ReturnStatement statement: statements) {
+                if (CancelSupport.getDefault().isCancelled()) {
+                    return;
+                }
+                Expression expression = statement.getExpression();
+                if (expression == null) {
+                    addHint(statement, Bundle.ReturnStatementWithoutValueHintErrorDesc(), hints);
+                }
+            }
+        }
+    }
+
     private boolean isNeverType(String name) {
         return getPhpVersion(fileObject).hasNeverType()
                 && Type.NEVER.equals(name);
     }
 
+   private boolean isVoidType(String name) {
+        return getPhpVersion(fileObject).hasVoidReturnType()
+                && Type.VOID.equals(name);

Review Comment:
   I am not sure, but I would expect that if we swap the conditions, the performance should be better (comparing strings should be faster than working with file objects).
   



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


[GitHub] [netbeans] troizet commented on a diff in pull request #5648: PHP: Adding a new hint ReturnStatementWithoutValueHintError.

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


##########
php/php.editor/src/org/netbeans/modules/php/editor/verification/ReturnTypeHintError.java:
##########
@@ -116,13 +118,18 @@ private void checkReturnType(Map<ASTNode, Set<ReturnStatement>> returnStatements
                 NamespaceName namespaceName = (NamespaceName) returnType;
                 String name = CodeUtils.extractUnqualifiedName(namespaceName);
                 checkVoidAndNeverReturnStatements(statements, name, hints);
+                checkReturnStatementsWithoutValue(statements, name, hints);
             } else if (returnType instanceof NullableType) {
                 Expression type = ((NullableType) returnType).getType();
                 if (type instanceof NamespaceName) {
                     NamespaceName namespaceName = (NamespaceName) type;
                     String name = CodeUtils.extractUnqualifiedName(namespaceName);
                     checkInvalidVoidAndNeverReturnType(type, name, hints);
-                }
+                } 
+                checkReturnStatementsWithoutValue(statements, "", hints);

Review Comment:
   Done.



##########
php/php.editor/src/org/netbeans/modules/php/editor/verification/ReturnTypeHintError.java:
##########
@@ -116,13 +118,18 @@ private void checkReturnType(Map<ASTNode, Set<ReturnStatement>> returnStatements
                 NamespaceName namespaceName = (NamespaceName) returnType;
                 String name = CodeUtils.extractUnqualifiedName(namespaceName);
                 checkVoidAndNeverReturnStatements(statements, name, hints);
+                checkReturnStatementsWithoutValue(statements, name, hints);
             } else if (returnType instanceof NullableType) {
                 Expression type = ((NullableType) returnType).getType();
                 if (type instanceof NamespaceName) {
                     NamespaceName namespaceName = (NamespaceName) type;
                     String name = CodeUtils.extractUnqualifiedName(namespaceName);
                     checkInvalidVoidAndNeverReturnType(type, name, hints);
-                }
+                } 
+                checkReturnStatementsWithoutValue(statements, "", hints);
+            } else if (returnType instanceof UnionType
+                    || returnType instanceof IntersectionType) {
+                checkReturnStatementsWithoutValue(statements, "", hints);

Review Comment:
   Done.



##########
php/php.editor/test/unit/data/testfiles/verification/ReturnTypeHintError/testReturnStatementWithoutValueHintError.php:
##########
@@ -0,0 +1,106 @@
+<?php

Review Comment:
   Done.



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


[GitHub] [netbeans] tmysik commented on pull request #5648: PHP: Adding a new error hint, as suggested in #5078.

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

   @junichi11 Not sure whether we need all the approvals; if not, feel free to merge anytime. 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


[GitHub] [netbeans] junichi11 commented on pull request #5648: PHP: Adding a new hint ReturnStatementWithoutValueHintError.

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

   @troizet Please implement it in `ReturnTypeHintError`.


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


[GitHub] [netbeans] junichi11 commented on pull request #5648: PHP: Adding a new hint ReturnStatementWithoutValueHintError.

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

   Please fix the commit message and 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


[GitHub] [netbeans] troizet commented on pull request #5648: PHP: Adding a new error hint, as suggested in #5078.

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

   Thanks to all!


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


[GitHub] [netbeans] troizet commented on pull request #5648: PHP: Adding a new hint ReturnStatementWithoutValueHintError.

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

   That's what I did at first. Then I decided it would be easier to do it separately. OK, I'll try again. 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


[GitHub] [netbeans] lbownik commented on a diff in pull request #5648: PHP: Adding a new hint ReturnStatementWithoutValueHintError.

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


##########
php/php.editor/src/org/netbeans/modules/php/editor/verification/ReturnStatementWithoutValueHintError.java:
##########
@@ -0,0 +1,218 @@
+/*
+ * 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.verification;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Set;
+import org.netbeans.api.annotations.common.NullAllowed;
+import org.netbeans.modules.csl.api.Hint;
+import org.netbeans.modules.csl.api.HintFix;
+import org.netbeans.modules.csl.api.OffsetRange;
+import org.netbeans.modules.csl.spi.support.CancelSupport;
+import org.netbeans.modules.php.api.PhpVersion;
+import org.netbeans.modules.php.editor.CodeUtils;
+import org.netbeans.modules.php.editor.model.FileScope;
+import org.netbeans.modules.php.editor.model.impl.Type;
+import org.netbeans.modules.php.editor.parser.PHPParseResult;
+import org.netbeans.modules.php.editor.parser.astnodes.ASTNode;
+import org.netbeans.modules.php.editor.parser.astnodes.Expression;
+import org.netbeans.modules.php.editor.parser.astnodes.FunctionDeclaration;
+import org.netbeans.modules.php.editor.parser.astnodes.IntersectionType;
+import org.netbeans.modules.php.editor.parser.astnodes.LambdaFunctionDeclaration;
+import org.netbeans.modules.php.editor.parser.astnodes.NamespaceName;
+import org.netbeans.modules.php.editor.parser.astnodes.NullableType;
+import org.netbeans.modules.php.editor.parser.astnodes.ReturnStatement;
+import org.netbeans.modules.php.editor.parser.astnodes.UnionType;
+import org.netbeans.modules.php.editor.parser.astnodes.visitors.DefaultVisitor;
+import org.openide.filesystems.FileObject;
+import org.openide.util.NbBundle;
+
+/**
+ * Checks if the return statement has a value if a return type is specified for the function
+ *
+ */
+public class ReturnStatementWithoutValueHintError extends HintErrorRule {
+
+    private FileObject fileObject;
+
+    @NbBundle.Messages("ReturnStatementWithoutValueHintErrorName=Return statement without value")
+    @Override
+    public String getDisplayName() {
+        return Bundle.ReturnStatementWithoutValueHintErrorName();
+    }
+
+    @Override
+    public void invoke(PHPRuleContext context, List<Hint> hints) {
+        PHPParseResult phpParseResult = (PHPParseResult) context.parserResult;
+        if (phpParseResult.getProgram() == null) {
+            return;
+        }
+        FileScope fileScope = context.fileScope;
+        fileObject = phpParseResult.getSnapshot().getSource().getFileObject();
+        if (fileScope != null && fileObject != null && appliesTo(fileObject)) {
+            if (CancelSupport.getDefault().isCancelled()) {
+                return;
+            }
+            CheckVisitor checkVisitor = new CheckVisitor();
+            phpParseResult.getProgram().accept(checkVisitor);
+            Map<ASTNode, Set<ReturnStatement>> returnStatements = checkVisitor.getReturnStatements();
+            checkReturnType(returnStatements, hints);
+        }
+    }
+
+    protected PhpVersion getPhpVersion(@NullAllowed FileObject file) {
+        if (file == null) {
+            return PhpVersion.getDefault();
+        }
+        return CodeUtils.getPhpVersion(file);
+    }
+
+    private boolean appliesTo(FileObject file) {
+        return getPhpVersion(file).compareTo(PhpVersion.PHP_70) >= 0;
+    }
+
+    private void checkReturnType(Map<ASTNode, Set<ReturnStatement>> returnStatements, List<Hint> hints) {
+        for (Entry<ASTNode, Set<ReturnStatement>> entry : returnStatements.entrySet()) {
+            if (CancelSupport.getDefault().isCancelled()) {
+                return;
+            }
+            ASTNode node = entry.getKey();
+            Expression returnType = null;
+            if (node instanceof FunctionDeclaration) {
+                returnType = ((FunctionDeclaration) node).getReturnType();
+            } else if (node instanceof LambdaFunctionDeclaration) {
+                returnType = ((LambdaFunctionDeclaration) node).getReturnType();
+            }
+
+            if (returnType == null) {
+                continue;
+            }
+            Set<ReturnStatement> statements = entry.getValue();
+
+            if (returnType instanceof NamespaceName) {
+                NamespaceName namespaceName = (NamespaceName) returnType;
+                String name = CodeUtils.extractUnqualifiedName(namespaceName);
+                checkReturnStatementsWithoutValue(statements, name, hints);
+            } else if (returnType instanceof NullableType
+                    || returnType instanceof UnionType
+                    || returnType instanceof IntersectionType) {
+                checkReturnStatementsWithoutValue(statements, "", hints);
+            }
+
+        }
+    }
+
+    @NbBundle.Messages({
+        "ReturnStatementWithoutValueHintErrorDesc=Return statement must be filled with the value"
+    })
+    private void checkReturnStatementsWithoutValue(Set<ReturnStatement> statements, String name, List<Hint> hints) {
+        if (!Type.VOID.equals(name) && !isNeverType(name)) {
+            // check empty return statement
+            statements.forEach((statement) -> {
+                if (CancelSupport.getDefault().isCancelled()) {
+                    return;

Review Comment:
   this will return from lambda, not from checkReturnStatementsWithoutValue
   is it intentional?



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


[GitHub] [netbeans] mbien commented on a diff in pull request #5648: PHP: Adding a new hint ReturnStatementWithoutValueHintError.

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


##########
php/php.editor/src/org/netbeans/modules/php/editor/verification/ReturnStatementWithoutValueHintError.java:
##########
@@ -0,0 +1,218 @@
+/*
+ * 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.verification;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Set;
+import org.netbeans.api.annotations.common.NullAllowed;
+import org.netbeans.modules.csl.api.Hint;
+import org.netbeans.modules.csl.api.HintFix;
+import org.netbeans.modules.csl.api.OffsetRange;
+import org.netbeans.modules.csl.spi.support.CancelSupport;
+import org.netbeans.modules.php.api.PhpVersion;
+import org.netbeans.modules.php.editor.CodeUtils;
+import org.netbeans.modules.php.editor.model.FileScope;
+import org.netbeans.modules.php.editor.model.impl.Type;
+import org.netbeans.modules.php.editor.parser.PHPParseResult;
+import org.netbeans.modules.php.editor.parser.astnodes.ASTNode;
+import org.netbeans.modules.php.editor.parser.astnodes.Expression;
+import org.netbeans.modules.php.editor.parser.astnodes.FunctionDeclaration;
+import org.netbeans.modules.php.editor.parser.astnodes.IntersectionType;
+import org.netbeans.modules.php.editor.parser.astnodes.LambdaFunctionDeclaration;
+import org.netbeans.modules.php.editor.parser.astnodes.NamespaceName;
+import org.netbeans.modules.php.editor.parser.astnodes.NullableType;
+import org.netbeans.modules.php.editor.parser.astnodes.ReturnStatement;
+import org.netbeans.modules.php.editor.parser.astnodes.UnionType;
+import org.netbeans.modules.php.editor.parser.astnodes.visitors.DefaultVisitor;
+import org.openide.filesystems.FileObject;
+import org.openide.util.NbBundle;
+
+/**
+ * Checks if the return statement has a value if a return type is specified for the function
+ *
+ */
+public class ReturnStatementWithoutValueHintError extends HintErrorRule {
+
+    private FileObject fileObject;
+
+    @NbBundle.Messages("ReturnStatementWithoutValueHintErrorName=Return statement without value")
+    @Override
+    public String getDisplayName() {
+        return Bundle.ReturnStatementWithoutValueHintErrorName();
+    }
+
+    @Override
+    public void invoke(PHPRuleContext context, List<Hint> hints) {
+        PHPParseResult phpParseResult = (PHPParseResult) context.parserResult;
+        if (phpParseResult.getProgram() == null) {
+            return;
+        }
+        FileScope fileScope = context.fileScope;
+        fileObject = phpParseResult.getSnapshot().getSource().getFileObject();
+        if (fileScope != null && fileObject != null && appliesTo(fileObject)) {
+            if (CancelSupport.getDefault().isCancelled()) {
+                return;
+            }
+            CheckVisitor checkVisitor = new CheckVisitor();
+            phpParseResult.getProgram().accept(checkVisitor);
+            Map<ASTNode, Set<ReturnStatement>> returnStatements = checkVisitor.getReturnStatements();
+            checkReturnType(returnStatements, hints);
+        }
+    }
+
+    protected PhpVersion getPhpVersion(@NullAllowed FileObject file) {
+        if (file == null) {
+            return PhpVersion.getDefault();
+        }
+        return CodeUtils.getPhpVersion(file);
+    }
+
+    private boolean appliesTo(FileObject file) {
+        return getPhpVersion(file).compareTo(PhpVersion.PHP_70) >= 0;
+    }
+
+    private void checkReturnType(Map<ASTNode, Set<ReturnStatement>> returnStatements, List<Hint> hints) {
+        for (Entry<ASTNode, Set<ReturnStatement>> entry : returnStatements.entrySet()) {
+            if (CancelSupport.getDefault().isCancelled()) {
+                return;
+            }
+            ASTNode node = entry.getKey();
+            Expression returnType = null;
+            if (node instanceof FunctionDeclaration) {
+                returnType = ((FunctionDeclaration) node).getReturnType();
+            } else if (node instanceof LambdaFunctionDeclaration) {
+                returnType = ((LambdaFunctionDeclaration) node).getReturnType();
+            }
+
+            if (returnType == null) {
+                continue;
+            }
+            Set<ReturnStatement> statements = entry.getValue();
+
+            if (returnType instanceof NamespaceName) {
+                NamespaceName namespaceName = (NamespaceName) returnType;
+                String name = CodeUtils.extractUnqualifiedName(namespaceName);
+                checkReturnStatementsWithoutValue(statements, name, hints);
+            } else if (returnType instanceof NullableType
+                    || returnType instanceof UnionType
+                    || returnType instanceof IntersectionType) {
+                checkReturnStatementsWithoutValue(statements, "", hints);
+            }
+
+        }
+    }
+
+    @NbBundle.Messages({
+        "ReturnStatementWithoutValueHintErrorDesc=Return statement must be filled with the value"
+    })
+    private void checkReturnStatementsWithoutValue(Set<ReturnStatement> statements, String name, List<Hint> hints) {
+        if (!Type.VOID.equals(name) && !isNeverType(name)) {
+            // check empty return statement
+            statements.forEach((statement) -> {
+                if (CancelSupport.getDefault().isCancelled()) {
+                    return;

Review Comment:
   this isn't bad performance wise since it would just loop without doing anything when canceled. But this can introduce bugs later e.g if someone adds something after the `foreach` loop.



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


[GitHub] [netbeans] tmysik commented on a diff in pull request #5648: PHP: Adding a new hint ReturnStatementWithoutValueHintError.

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


##########
php/php.editor/src/org/netbeans/modules/php/editor/verification/ReturnTypeHintError.java:
##########
@@ -133,17 +140,17 @@ private void checkReturnType(Map<ASTNode, Set<ReturnStatement>> returnStatements
         "ReturnTypeHintErrorVoidDesc=\"{0}\" cannot return anything"
     })
     private void checkVoidAndNeverReturnStatements(Set<ReturnStatement> statements, String name, List<Hint> hints) {
-        if (Type.VOID.equals(name) || isNeverType(name)) {
+        if (isVoidType(name) || isNeverType(name)) {
             // check empty return statement
-            statements.forEach((statement) -> {
+            for (ReturnStatement statement: statements) {

Review Comment:
   I think I see now - because of the `CancelSupport`, right?
   



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


[GitHub] [netbeans] troizet commented on a diff in pull request #5648: PHP: Adding a new hint ReturnStatementWithoutValueHintError.

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


##########
php/php.editor/src/org/netbeans/modules/php/editor/verification/ReturnStatementWithoutValueHintError.java:
##########
@@ -0,0 +1,218 @@
+/*
+ * 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.verification;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Set;
+import org.netbeans.api.annotations.common.NullAllowed;
+import org.netbeans.modules.csl.api.Hint;
+import org.netbeans.modules.csl.api.HintFix;
+import org.netbeans.modules.csl.api.OffsetRange;
+import org.netbeans.modules.csl.spi.support.CancelSupport;
+import org.netbeans.modules.php.api.PhpVersion;
+import org.netbeans.modules.php.editor.CodeUtils;
+import org.netbeans.modules.php.editor.model.FileScope;
+import org.netbeans.modules.php.editor.model.impl.Type;
+import org.netbeans.modules.php.editor.parser.PHPParseResult;
+import org.netbeans.modules.php.editor.parser.astnodes.ASTNode;
+import org.netbeans.modules.php.editor.parser.astnodes.Expression;
+import org.netbeans.modules.php.editor.parser.astnodes.FunctionDeclaration;
+import org.netbeans.modules.php.editor.parser.astnodes.IntersectionType;
+import org.netbeans.modules.php.editor.parser.astnodes.LambdaFunctionDeclaration;
+import org.netbeans.modules.php.editor.parser.astnodes.NamespaceName;
+import org.netbeans.modules.php.editor.parser.astnodes.NullableType;
+import org.netbeans.modules.php.editor.parser.astnodes.ReturnStatement;
+import org.netbeans.modules.php.editor.parser.astnodes.UnionType;
+import org.netbeans.modules.php.editor.parser.astnodes.visitors.DefaultVisitor;
+import org.openide.filesystems.FileObject;
+import org.openide.util.NbBundle;
+
+/**
+ * Checks if the return statement has a value if a return type is specified for the function
+ *
+ */
+public class ReturnStatementWithoutValueHintError extends HintErrorRule {
+
+    private FileObject fileObject;
+
+    @NbBundle.Messages("ReturnStatementWithoutValueHintErrorName=Return statement without value")
+    @Override
+    public String getDisplayName() {
+        return Bundle.ReturnStatementWithoutValueHintErrorName();
+    }
+
+    @Override
+    public void invoke(PHPRuleContext context, List<Hint> hints) {
+        PHPParseResult phpParseResult = (PHPParseResult) context.parserResult;
+        if (phpParseResult.getProgram() == null) {
+            return;
+        }
+        FileScope fileScope = context.fileScope;
+        fileObject = phpParseResult.getSnapshot().getSource().getFileObject();
+        if (fileScope != null && fileObject != null && appliesTo(fileObject)) {
+            if (CancelSupport.getDefault().isCancelled()) {
+                return;
+            }
+            CheckVisitor checkVisitor = new CheckVisitor();
+            phpParseResult.getProgram().accept(checkVisitor);
+            Map<ASTNode, Set<ReturnStatement>> returnStatements = checkVisitor.getReturnStatements();
+            checkReturnType(returnStatements, hints);
+        }
+    }
+
+    protected PhpVersion getPhpVersion(@NullAllowed FileObject file) {
+        if (file == null) {
+            return PhpVersion.getDefault();
+        }
+        return CodeUtils.getPhpVersion(file);
+    }
+
+    private boolean appliesTo(FileObject file) {
+        return getPhpVersion(file).compareTo(PhpVersion.PHP_70) >= 0;
+    }
+
+    private void checkReturnType(Map<ASTNode, Set<ReturnStatement>> returnStatements, List<Hint> hints) {
+        for (Entry<ASTNode, Set<ReturnStatement>> entry : returnStatements.entrySet()) {
+            if (CancelSupport.getDefault().isCancelled()) {
+                return;
+            }
+            ASTNode node = entry.getKey();
+            Expression returnType = null;
+            if (node instanceof FunctionDeclaration) {
+                returnType = ((FunctionDeclaration) node).getReturnType();
+            } else if (node instanceof LambdaFunctionDeclaration) {
+                returnType = ((LambdaFunctionDeclaration) node).getReturnType();
+            }
+
+            if (returnType == null) {
+                continue;
+            }
+            Set<ReturnStatement> statements = entry.getValue();
+
+            if (returnType instanceof NamespaceName) {
+                NamespaceName namespaceName = (NamespaceName) returnType;
+                String name = CodeUtils.extractUnqualifiedName(namespaceName);
+                checkReturnStatementsWithoutValue(statements, name, hints);
+            } else if (returnType instanceof NullableType
+                    || returnType instanceof UnionType
+                    || returnType instanceof IntersectionType) {
+                checkReturnStatementsWithoutValue(statements, "", hints);
+            }
+
+        }
+    }
+
+    @NbBundle.Messages({
+        "ReturnStatementWithoutValueHintErrorDesc=Return statement must be filled with the value"
+    })
+    private void checkReturnStatementsWithoutValue(Set<ReturnStatement> statements, String name, List<Hint> hints) {
+        if (!Type.VOID.equals(name) && !isNeverType(name)) {
+            // check empty return statement
+            statements.forEach((statement) -> {
+                if (CancelSupport.getDefault().isCancelled()) {
+                    return;

Review Comment:
   I got this code from here [php/php.editor/src/org/netbeans/modules/php/editor/verification/ReturnTypeHintError.java](https://github.com/apache/netbeans/blob/cef11a8c61656b7e15562a9d4c28e9d386a1d7f1/php/php.editor/src/org/netbeans/modules/php/editor/verification/ReturnTypeHintError.java#L135-L148).
   Is it wrong?



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


[GitHub] [netbeans] lbownik commented on a diff in pull request #5648: PHP: Adding a new hint ReturnStatementWithoutValueHintError.

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


##########
php/php.editor/src/org/netbeans/modules/php/editor/verification/ReturnStatementWithoutValueHintError.java:
##########
@@ -0,0 +1,218 @@
+/*
+ * 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.verification;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Set;
+import org.netbeans.api.annotations.common.NullAllowed;
+import org.netbeans.modules.csl.api.Hint;
+import org.netbeans.modules.csl.api.HintFix;
+import org.netbeans.modules.csl.api.OffsetRange;
+import org.netbeans.modules.csl.spi.support.CancelSupport;
+import org.netbeans.modules.php.api.PhpVersion;
+import org.netbeans.modules.php.editor.CodeUtils;
+import org.netbeans.modules.php.editor.model.FileScope;
+import org.netbeans.modules.php.editor.model.impl.Type;
+import org.netbeans.modules.php.editor.parser.PHPParseResult;
+import org.netbeans.modules.php.editor.parser.astnodes.ASTNode;
+import org.netbeans.modules.php.editor.parser.astnodes.Expression;
+import org.netbeans.modules.php.editor.parser.astnodes.FunctionDeclaration;
+import org.netbeans.modules.php.editor.parser.astnodes.IntersectionType;
+import org.netbeans.modules.php.editor.parser.astnodes.LambdaFunctionDeclaration;
+import org.netbeans.modules.php.editor.parser.astnodes.NamespaceName;
+import org.netbeans.modules.php.editor.parser.astnodes.NullableType;
+import org.netbeans.modules.php.editor.parser.astnodes.ReturnStatement;
+import org.netbeans.modules.php.editor.parser.astnodes.UnionType;
+import org.netbeans.modules.php.editor.parser.astnodes.visitors.DefaultVisitor;
+import org.openide.filesystems.FileObject;
+import org.openide.util.NbBundle;
+
+/**
+ * Checks if the return statement has a value if a return type is specified for the function
+ *
+ */
+public class ReturnStatementWithoutValueHintError extends HintErrorRule {
+
+    private FileObject fileObject;
+
+    @NbBundle.Messages("ReturnStatementWithoutValueHintErrorName=Return statement without value")
+    @Override
+    public String getDisplayName() {
+        return Bundle.ReturnStatementWithoutValueHintErrorName();
+    }
+
+    @Override
+    public void invoke(PHPRuleContext context, List<Hint> hints) {
+        PHPParseResult phpParseResult = (PHPParseResult) context.parserResult;
+        if (phpParseResult.getProgram() == null) {
+            return;
+        }
+        FileScope fileScope = context.fileScope;
+        fileObject = phpParseResult.getSnapshot().getSource().getFileObject();
+        if (fileScope != null && fileObject != null && appliesTo(fileObject)) {
+            if (CancelSupport.getDefault().isCancelled()) {
+                return;
+            }
+            CheckVisitor checkVisitor = new CheckVisitor();
+            phpParseResult.getProgram().accept(checkVisitor);
+            Map<ASTNode, Set<ReturnStatement>> returnStatements = checkVisitor.getReturnStatements();
+            checkReturnType(returnStatements, hints);
+        }
+    }
+
+    protected PhpVersion getPhpVersion(@NullAllowed FileObject file) {
+        if (file == null) {
+            return PhpVersion.getDefault();
+        }
+        return CodeUtils.getPhpVersion(file);
+    }
+
+    private boolean appliesTo(FileObject file) {
+        return getPhpVersion(file).compareTo(PhpVersion.PHP_70) >= 0;
+    }
+
+    private void checkReturnType(Map<ASTNode, Set<ReturnStatement>> returnStatements, List<Hint> hints) {
+        for (Entry<ASTNode, Set<ReturnStatement>> entry : returnStatements.entrySet()) {
+            if (CancelSupport.getDefault().isCancelled()) {
+                return;
+            }
+            ASTNode node = entry.getKey();
+            Expression returnType = null;
+            if (node instanceof FunctionDeclaration) {
+                returnType = ((FunctionDeclaration) node).getReturnType();
+            } else if (node instanceof LambdaFunctionDeclaration) {
+                returnType = ((LambdaFunctionDeclaration) node).getReturnType();
+            }
+
+            if (returnType == null) {
+                continue;
+            }
+            Set<ReturnStatement> statements = entry.getValue();
+
+            if (returnType instanceof NamespaceName) {
+                NamespaceName namespaceName = (NamespaceName) returnType;
+                String name = CodeUtils.extractUnqualifiedName(namespaceName);
+                checkReturnStatementsWithoutValue(statements, name, hints);
+            } else if (returnType instanceof NullableType
+                    || returnType instanceof UnionType
+                    || returnType instanceof IntersectionType) {
+                checkReturnStatementsWithoutValue(statements, "", hints);
+            }
+
+        }
+    }
+
+    @NbBundle.Messages({
+        "ReturnStatementWithoutValueHintErrorDesc=Return statement must be filled with the value"
+    })
+    private void checkReturnStatementsWithoutValue(Set<ReturnStatement> statements, String name, List<Hint> hints) {
+        if (!Type.VOID.equals(name) && !isNeverType(name)) {
+            // check empty return statement
+            statements.forEach((statement) -> {
+                if (CancelSupport.getDefault().isCancelled()) {
+                    return;

Review Comment:
   I don't know. I just spotted a strange costruct and brought it to your attention. Maybe it is perfectly fine.



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


[GitHub] [netbeans] junichi11 commented on a diff in pull request #5648: PHP: Adding a new hint ReturnStatementWithoutValueHintError.

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


##########
php/php.editor/src/org/netbeans/modules/php/editor/verification/ReturnTypeHintError.java:
##########
@@ -116,13 +118,18 @@ private void checkReturnType(Map<ASTNode, Set<ReturnStatement>> returnStatements
                 NamespaceName namespaceName = (NamespaceName) returnType;
                 String name = CodeUtils.extractUnqualifiedName(namespaceName);
                 checkVoidAndNeverReturnStatements(statements, name, hints);
+                checkReturnStatementsWithoutValue(statements, name, hints);
             } else if (returnType instanceof NullableType) {
                 Expression type = ((NullableType) returnType).getType();
                 if (type instanceof NamespaceName) {
                     NamespaceName namespaceName = (NamespaceName) type;
                     String name = CodeUtils.extractUnqualifiedName(namespaceName);
                     checkInvalidVoidAndNeverReturnType(type, name, hints);
-                }
+                } 
+                checkReturnStatementsWithoutValue(statements, "", hints);

Review Comment:
   `// NOI18N`



##########
php/php.editor/src/org/netbeans/modules/php/editor/verification/ReturnTypeHintError.java:
##########
@@ -116,13 +118,18 @@ private void checkReturnType(Map<ASTNode, Set<ReturnStatement>> returnStatements
                 NamespaceName namespaceName = (NamespaceName) returnType;
                 String name = CodeUtils.extractUnqualifiedName(namespaceName);
                 checkVoidAndNeverReturnStatements(statements, name, hints);
+                checkReturnStatementsWithoutValue(statements, name, hints);
             } else if (returnType instanceof NullableType) {
                 Expression type = ((NullableType) returnType).getType();
                 if (type instanceof NamespaceName) {
                     NamespaceName namespaceName = (NamespaceName) type;
                     String name = CodeUtils.extractUnqualifiedName(namespaceName);
                     checkInvalidVoidAndNeverReturnType(type, name, hints);
-                }
+                } 
+                checkReturnStatementsWithoutValue(statements, "", hints);
+            } else if (returnType instanceof UnionType
+                    || returnType instanceof IntersectionType) {
+                checkReturnStatementsWithoutValue(statements, "", hints);

Review Comment:
   `// NOI18N`



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


[GitHub] [netbeans] junichi11 commented on a diff in pull request #5648: PHP: Adding a new hint ReturnStatementWithoutValueHintError.

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


##########
php/php.editor/test/unit/data/testfiles/verification/ReturnTypeHintError/testReturnStatementWithoutValueHintError.php:
##########
@@ -0,0 +1,106 @@
+<?php

Review Comment:
   Please add the license header.



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


[GitHub] [netbeans] tmysik commented on a diff in pull request #5648: PHP: Adding a new hint ReturnStatementWithoutValueHintError.

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


##########
php/php.editor/src/org/netbeans/modules/php/editor/verification/ReturnTypeHintError.java:
##########
@@ -152,17 +159,40 @@ private void checkVoidAndNeverReturnStatements(Set<ReturnStatement> statements,
         "ReturnTypeHintErrorInvalidVoidDesc=\"{0}\" cannot be used with \"?\""
     })
     private void checkInvalidVoidAndNeverReturnType(Expression returnType, String name, List<Hint> hints) {
-        if (Type.VOID.equals(name)
+        if (isVoidType(name)
                 || isNeverType(name)) {
             addHint(returnType, Bundle.ReturnTypeHintErrorInvalidVoidDesc(name), hints);
         }
     }
 
+    @NbBundle.Messages({
+        "ReturnStatementWithoutValueHintErrorDesc=Return statement must be filled with the value"
+    })
+    private void checkReturnStatementsWithoutValue(Set<ReturnStatement> statements, String name, List<Hint> hints) {
+        if (!isVoidType(name) && !isNeverType(name)) {
+            // check empty return statement
+            for (ReturnStatement statement: statements) {
+                if (CancelSupport.getDefault().isCancelled()) {
+                    return;
+                }
+                Expression expression = statement.getExpression();
+                if (expression == null) {
+                    addHint(statement, Bundle.ReturnStatementWithoutValueHintErrorDesc(), hints);
+                }
+            }
+        }
+    }
+
     private boolean isNeverType(String name) {
         return getPhpVersion(fileObject).hasNeverType()
                 && Type.NEVER.equals(name);
     }
 
+   private boolean isVoidType(String name) {
+        return getPhpVersion(fileObject).hasVoidReturnType()
+                && Type.VOID.equals(name);

Review Comment:
   (It applies also for `isNeverType` 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


[GitHub] [netbeans] tmysik commented on a diff in pull request #5648: PHP: Adding a new hint ReturnStatementWithoutValueHintError.

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


##########
php/php.editor/src/org/netbeans/modules/php/editor/verification/ReturnTypeHintError.java:
##########
@@ -116,13 +118,18 @@ private void checkReturnType(Map<ASTNode, Set<ReturnStatement>> returnStatements
                 NamespaceName namespaceName = (NamespaceName) returnType;
                 String name = CodeUtils.extractUnqualifiedName(namespaceName);
                 checkVoidAndNeverReturnStatements(statements, name, hints);
+                checkReturnStatementsWithoutValue(statements, name, hints);
             } else if (returnType instanceof NullableType) {
                 Expression type = ((NullableType) returnType).getType();
                 if (type instanceof NamespaceName) {
                     NamespaceName namespaceName = (NamespaceName) type;
                     String name = CodeUtils.extractUnqualifiedName(namespaceName);
                     checkInvalidVoidAndNeverReturnType(type, name, hints);
-                }
+                } 

Review Comment:
   Remove the trailing whitespace.
   



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