linter: added checks for functions #605
Conversation
Added check for bare returns for functions with non-void return types. Added check for the absence of return statement in function with @return annotation, which is not equal to void. For this, the storage of CurrentFunction has been improved. CurrentFunction now stores the fully qualified name of the function with its class and namespace. A separate CurrentMethod field was created to store the names of the methods.
| // Required to make a decision in void vs null type selection, | ||
| // since "return" is the same as "return null". | ||
| bareReturn bool | ||
| // when the function contains at least one return or yield. | ||
| containsReturnOrYield bool |
There was a problem hiding this comment.
exitFlags should already contain this information in the way you want?
There was a problem hiding this comment.
I need to know if there is a return or yield in the body of the function in order to accurately determine "mismatchingReturn". I'm not sure that exitFlags holds the information I need. Comment mentions other things // if function has exit / die / throw, then ExitFlags will be <> 0.
| @@ -698,7 +698,12 @@ func (d *RootWalker) handleFuncStmts(params []meta.FuncParam, uses, stmts []node | |||
| prematureExitFlags = cleanFlags | |||
| } | |||
|
|
|||
| // if the function contains only one statement and this statement is throw. | |||
| oneThrowStatementFunction := b.containsThrow && len(stmts) == 1 | |||
There was a problem hiding this comment.
Why not two? Ten? Why this special case only?
There was a problem hiding this comment.
in basic_test.go:
// Functions of this kind are very common in Symfony, so it makes sense not to consider this
// a loss of return, although in fact the @return annotation is not entirely correct.
| @@ -52,6 +52,21 @@ MAYBE phpdoc: Missing PHPDoc for "getHelperName" public method at testdata/mus | |||
| MAYBE phpdoc: Missing PHPDoc for "getTemplateName" public method at testdata/mustache/src/Mustache/Exception/UnknownTemplateException.php:34 | |||
| public function getTemplateName() | |||
| ^^^^^^^^^^^^^^^ | |||
| WARNING bareReturn: Replace 'return' with 'return null' at testdata/mustache/src/Mustache/Parser.php:207 | |||
There was a problem hiding this comment.
return; and return null; already mean the same thing. Maybe provide more context here on why exactly do you ask to replace it in this function?
There was a problem hiding this comment.
Probably we can provide additional information after the first sentence?
For example:
<critical> WARNING bareReturn: Replace 'return' with 'return null'. The '\f' function has @return annotation with non-'void' value, so 'return null' is preferred. at C:/projects/noverify/example/index.php:7
return;
^^^^^^^
The message for the 'bareReturn' error has been expanded. Added comment why replacement is needed. Fixed crash bug baseline-test.
Removed the unnecessary check for the '\r' suffix in the report function.
|
Closed until better times. |
@returnannotation, which is not equal to void. Find mismatching @return annotations #561For this, the storage of
CurrentFunctionhas been improved.CurrentFunctionnow stores the fully qualified name of the function with its class and namespace. A separateCurrentMethodfield was created to store the names of the methods. #597Fixes #561
Fixes #563
Fixes #597