-
Notifications
You must be signed in to change notification settings - Fork 437
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Increase phpstan level to 8 #391
Conversation
75d88fc
to
49957fe
Compare
.php_cs.cache
Outdated
@@ -0,0 +1 @@ | |||
{"php":"7.4.2","version":"2.16.1:v2.16.1#c8afb599858876e95e8ebfcd97812d383fa23f02","indent":" ","lineEnding":"\n","rules":{"visibility_required":true,"ternary_to_null_coalescing":true,"encoding":true,"full_opening_tag":true,"blank_line_after_namespace":true,"braces":true,"class_definition":true,"constant_case":true,"elseif":true,"function_declaration":true,"indentation_type":true,"line_ending":true,"lowercase_keywords":true,"method_argument_space":{"on_multiline":"ensure_fully_multiline"},"no_break_comment":true,"no_closing_tag":true,"no_spaces_after_function_name":true,"no_spaces_inside_parenthesis":true,"no_trailing_whitespace":true,"no_trailing_whitespace_in_comment":true,"single_blank_line_at_eof":true,"single_class_element_per_statement":{"elements":["property"]},"single_import_per_statement":true,"single_line_after_imports":true,"switch_case_semicolon_to_colon":true,"switch_case_space":true,"array_indentation":true,"array_syntax":{"syntax":"short"},"align_multiline_comment":{"comment_type":"all_multiline"},"binary_operator_spaces":true,"blank_line_after_opening_tag":true,"blank_line_before_statement":true,"cast_spaces":true,"class_attributes_separation":true,"combine_consecutive_issets":true,"combine_consecutive_unsets":true,"compact_nullable_typehint":true,"concat_space":{"spacing":"one"},"PedroTroller\/exceptions_punctuation":true,"PedroTroller\/line_break_between_method_arguments":{"max-args":10},"PedroTroller\/comment_line_to_phpdoc_block":true,"PedroTroller\/useless_code_after_return":true},"hashes":{"src\/Knp\/Snappy\/AbstractGenerator.php":3020002203,"src\/Knp\/Snappy\/GeneratorInterface.php":3708897097,"src\/Knp\/Snappy\/Image.php":207790329,"src\/Knp\/Snappy\/Pdf.php":2353233392,"src\/Knp\/Snappy\/Exception\/FileAlreadyExistsException.php":1655581840,"tests\/Knp\/Snappy\/AbstractGeneratorTest.php":2344254468,"tests\/Knp\/Snappy\/PdfTest.php":156588972,"tests\/Knp\/Snappy\/ImageTest.php":4066889974}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please remove this file from this PR and add it to the .gitignore
;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😱
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add this file to the .gitignore
file please ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not related to this PR, I'm adding the CSFixer on a different PR and it has already been added on .gitignore
there 😉
It has erroneously been added during a branch switch 😞
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Just one small issue IMHO.
.php_cs.cache
Outdated
@@ -0,0 +1 @@ | |||
{"php":"7.4.2","version":"2.16.1:v2.16.1#c8afb599858876e95e8ebfcd97812d383fa23f02","indent":" ","lineEnding":"\n","rules":{"visibility_required":true,"ternary_to_null_coalescing":true,"encoding":true,"full_opening_tag":true,"blank_line_after_namespace":true,"braces":true,"class_definition":true,"constant_case":true,"elseif":true,"function_declaration":true,"indentation_type":true,"line_ending":true,"lowercase_keywords":true,"method_argument_space":{"on_multiline":"ensure_fully_multiline"},"no_break_comment":true,"no_closing_tag":true,"no_spaces_after_function_name":true,"no_spaces_inside_parenthesis":true,"no_trailing_whitespace":true,"no_trailing_whitespace_in_comment":true,"single_blank_line_at_eof":true,"single_class_element_per_statement":{"elements":["property"]},"single_import_per_statement":true,"single_line_after_imports":true,"switch_case_semicolon_to_colon":true,"switch_case_space":true,"array_indentation":true,"array_syntax":{"syntax":"short"},"align_multiline_comment":{"comment_type":"all_multiline"},"binary_operator_spaces":true,"blank_line_after_opening_tag":true,"blank_line_before_statement":true,"cast_spaces":true,"class_attributes_separation":true,"combine_consecutive_issets":true,"combine_consecutive_unsets":true,"compact_nullable_typehint":true,"concat_space":{"spacing":"one"},"PedroTroller\/exceptions_punctuation":true,"PedroTroller\/line_break_between_method_arguments":{"max-args":10},"PedroTroller\/comment_line_to_phpdoc_block":true,"PedroTroller\/useless_code_after_return":true},"hashes":{"src\/Knp\/Snappy\/AbstractGenerator.php":3020002203,"src\/Knp\/Snappy\/GeneratorInterface.php":3708897097,"src\/Knp\/Snappy\/Image.php":207790329,"src\/Knp\/Snappy\/Pdf.php":2353233392,"src\/Knp\/Snappy\/Exception\/FileAlreadyExistsException.php":1655581840,"tests\/Knp\/Snappy\/AbstractGeneratorTest.php":2344254468,"tests\/Knp\/Snappy\/PdfTest.php":156588972,"tests\/Knp\/Snappy\/ImageTest.php":4066889974}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m not sure it’s a good idea to commit cache files to the repo 😉 Maybe you can remove it, and add a *.cache
rule to the .gitignore
file in order to avoid this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not related to this PR, I'm adding the CSFixer on a different PR and it has already been added on .gitignore there 😉
It has erroneously been added during a branch switch 😞
$php_executable = $path . DIRECTORY_SEPARATOR . 'php' . (isset($_SERVER['WINDIR']) ? '.exe' : ''); | ||
if (file_exists($php_executable) && is_file($php_executable)) { | ||
return $php_executable; | ||
if (false !== getenv('PATH')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I generally try to avoid nesting whole blocks of code in a if
block (it can hinder readability and increase cyclomatic complexity) if it can be done in another way; here, I think you can use the early return technique:
if (false === getenv('PATH')) {
return null;
}
5e04f87
to
15d05f8
Compare
The error
Parameter #1 $command of class Symfony\Component\Process\Process constructor expects array, string given.
has been ignored because theSymfony\Component\Process\Process
constructor is only called with a string as its first parameter on v3.4 (see the condition above) and on that version, calling it with a string it's still supported (that part of the code will be removed when the support for Symfony 3.4 will be removed).