private function checkIfElseStatements(\PHPParser_Node_Stmt_If $node, BlockNode $if, BlockNode $else) { $ifTrue = $this->returnsTrue($if); $elseTrue = $this->returnsTrue($else); if ($ifTrue === $elseTrue) { if (null === $node->else) { $this->phpFile->addComment($node->getLine(), Comment::error('suspicious_code.superfluous_if_return', 'This ``if`` statement and the following ``return`` statement are superfluous as you return always ``%return_value%``.', array('return_value' => $ifTrue ? 'true' : 'false'))); } else { $this->phpFile->addComment($node->getLine(), Comment::error('suspicious_code.superfluous_if_else', 'This ``if``-``else`` statement are superfluous as you return always ``%return_value%``.', array('return_value' => $ifTrue ? 'true' : 'false'))); } return; } $condType = $node->cond->getAttribute('type'); $possiblyNeedsCast = !$condType || !$condType->isBooleanType(); $cond = $node->cond; if (!$ifTrue) { if ($cond instanceof \PHPParser_Node_Expr_BooleanNot) { $cond = $cond->expr; $condType = $cond->getAttribute('type'); $possiblyNeedsCast = !$condType || !$condType->isBooleanType(); } else { $cond = new \PHPParser_Node_Expr_BooleanNot($cond); $possiblyNeedsCast = false; } } if ($possiblyNeedsCast) { $cond = new \PHPParser_Node_Expr_Cast_Bool($cond); } if (null !== $node->else) { $this->phpFile->addComment($node->getLine(), Comment::warning('coding_style.simplify_boolean_return_of_if_else', 'The ``if``-``else`` statement can be simplified to ``return %expr%;``.', array('expr' => self::$prettyPrinter->prettyPrintExpr($cond)))); } else { $this->phpFile->addComment($node->getLine(), Comment::warning('coding_style.simplify_boolean_return_of_if_return', 'This ``if`` statement, and the following ``return`` statement can be replaced with ``return %expr%;``.', array('expr' => self::$prettyPrinter->prettyPrintExpr($cond)))); } }
public function visit(NodeTraversal $t, \PHPParser_Node $n, \PHPParser_Node $parent = null) { if ($n instanceof \PHPParser_Node_Stmt_Property) { $containerNode = NodeUtil::getContainer($n)->get(); $class = $this->typeRegistry->getClassByNode($containerNode); if ($class->isInterface()) { $this->phpFile->addComment($n->getLine(), Comment::error('basic_semantics.property_on_interface', 'In PHP, declaring a method on an interface is not yet allowed.')); } } if (NodeUtil::isMethodContainer($n)) { $class = $this->typeRegistry->getClassByNode($n); if ($class instanceof Clazz && !$class->isAbstract()) { $abstractMethods = array(); foreach ($class->getMethods() as $method) { assert($method instanceof ClassMethod); /** @var $method ClassMethod */ if ($method->isAbstract()) { $abstractMethods[] = $method->getMethod()->getName(); } } if (!empty($abstractMethods)) { switch (count($abstractMethods)) { case 1: $this->phpFile->addComment($n->getLine(), Comment::error('basic_semantics.abstract_method_on_non_abstract_class', 'There is one abstract method ``%method%`` in this class; you could implement it, or declare this class as abstract.', array('method' => reset($abstractMethods)))); break; default: $this->phpFile->addComment($n->getLine(), Comment::error('basic_semantics.abstract_methods_on_non_abstract_class', 'There is at least one abstract method in this class. Maybe declare it as abstract, or implement the remaining methods: %methods%', array('methods' => implode(', ', $abstractMethods)))); } } } } }
public function testAdd() { $col = new \Scrutinizer\PhpAnalyzer\Model\CommentCollection(); $a = \Scrutinizer\PhpAnalyzer\Model\Comment::error('foo', 'bar'); $b = \Scrutinizer\PhpAnalyzer\Model\Comment::error('foo', 'bar'); $col->add(1, $a); $col->add(1, $b); $this->assertCount(1, $col->all(1)); }
private static function createPhpFile($name, $content) { $file = new PhpFile($name, $content); try { $ast = self::$phpParser->parse(new \PHPParser_Lexer($content)); } catch (\PHPParser_Error $parserEx) { // This at least allows to run all the passes. For those that // need an AST to work, they will obviously not do anything // useful, but maybe some can. // TODO: Implement some heuristics to attempt to fix the code. $ast = array(new BlockNode(array())); $lineNb = $parserEx->getRawLine(); if (-1 === $lineNb) { // This is such a serious error that we at least need to // report it somehow even if the line is off. $file->addComment(1, Comment::error('unparsable_code', "Cannot point out a specific line, but got the following parsing error when trying this code:\n\n%message%", array('message' => $parserEx->getRawMessage()))); } else { $file->addComment($lineNb, Comment::error('unparsable_code_with_line', "This code did not parse for me. Apparently, there is an error somewhere around this line:\n\n%message%", array('message' => $parserEx->getRawMessage()))); } } $traverser = new \PHPParser_NodeTraverser(); $traverser->addVisitor(new \PHPParser_NodeVisitor_NameResolver()); $traverser->addVisitor(new NormalizingNodeVisitor()); $ast = $traverser->traverse($ast); // Wrap the AST in a block node if it has more than one root if (count($ast) > 1) { $ast = array(new BlockNode($ast)); } else { if (0 === count($ast)) { $ast = array(new BlockNode(array())); } } $traverser = new \PHPParser_NodeTraverser(); $traverser->addVisitor(new \PHPParser_NodeVisitor_NodeConnector()); // do not assign the AST here as the traverser clones nodes resulting // in different references for "parent", "next", "previous", etc. $traverser->traverse($ast); $file->setAst($ast[0]); return $file; }
public function visit(NodeTraversal $t, \PHPParser_Node $node, \PHPParser_Node $parent = null) { if (!NodeUtil::isCallLike($node)) { return; } if (null === ($function = $this->typeRegistry->getCalledFunctionByNode($node))) { return; } foreach ($function->getParameters() as $param) { if (!$param->isPassedByRef()) { continue; } $index = $param->getIndex(); if (!isset($node->args[$index])) { continue; } $arg = $node->args[$index]; if (!NodeUtil::canBePassedByRef($arg->value)) { $this->phpFile->addComment($arg->getLine(), Comment::error('usage.non_referencable_arg', '``%argument%`` cannot be passed to ``%function_name%()`` as the parameter ``$%parameter_name%`` expects a reference.', array('argument' => self::$prettyPrinter->prettyPrintExpr($arg->value), 'function_name' => $function->getName(), 'parameter_name' => $param->getName()))); } } }
private function handleUse(\PHPParser_Node_Stmt_UseUse $use) { if (!$this->getSetting('use_statement_alias_conflict')) { return; } $namespace = null; $parent = $use->getAttribute('parent'); while (null !== $parent) { if ($parent instanceof \PHPParser_Node_Stmt_Namespace) { $namespace = $parent->name ? implode('\\', $parent->name->parts) : null; break; } $parent = $parent->getAttribute('parent'); } $localClass = ($namespace ? $namespace . '\\' : '') . $use->alias; if (strtolower($localClass) === strtolower(implode('\\', $use->name->parts))) { return; } if ($this->typeRegistry->hasClass($localClass)) { $this->phpFile->addComment($use->name->getLine(), Comment::error('suspicious_code.use_statement_alias_conflict', 'This use statement conflicts with another class in this namespace, ``%local_class%``.', array('local_class' => $localClass))); } }
private function checkForMissingProperty(\PHPParser_Node_Expr_PropertyFetch $node) { if (!is_string($node->name)) { return; } if (!($objType = $node->var->getAttribute('type'))) { return; } $objType = $objType->restrictByNotNull()->toMaybeObjectType(); if (!$objType) { // TODO: Add support to check on union types. return; } if ($objType->isInterface()) { $this->phpFile->addComment($node->getLine(), Comment::warning('types.property_access_on_interface', 'Accessing "%property_name%" on the interface "%interface_name%" suggest that you code against a concrete implementation. How about adding an ``instanceof`` check?', array('property_name' => $node->name, 'interface_name' => $objType->getName()))); return; } if (!$objType->isNormalized() || $objType->hasProperty($node->name)) { return; } // Ignore all property accesses on ``stdClass`` objects. Currently, we have // no way to describe, or track RECORD types. As such, any messages related to // stdClass are likely wrong, but at least there are too many false-positives // for now. So, we just disable this. if ($objType->isSubtypeOf($this->typeRegistry->getClassOrCreate('stdClass'))) { return; } // Ignore all property reads on ``SimpleXMLElement`` objects. The reasoning // behind this is similar to the exception for ``stdClass`` objects above. // We simply have no reliable way to describe the structure of these objects // for now. So, we disable checks for them to avoid a flood of false-positives. if (!\Scrutinizer\PhpAnalyzer\PhpParser\NodeUtil::isAssignmentOp($node->getAttribute('parent')) && $objType->isSubtypeOf($this->typeRegistry->getClassOrCreate('SimpleXMLElement'))) { return; } // Property accesses inside isset() are safe, do not make any noise just yet. if ($node->getAttribute('parent') instanceof \PHPParser_Node_Expr_Isset) { return; } if (null !== ($bestName = $this->getMostSimilarName($node->name, $objType->getPropertyNames()))) { $this->phpFile->addComment($node->getLine(), Comment::error('typos.mispelled_property_name', 'The property "%offending_property_name%" does not exist. Did you mean "%closest_property_name%"?', array('offending_property_name' => $node->name, 'closest_property_name' => $bestName))); } else { // Ignore additional accesses to this property. $objType->addProperty(new Property($node->name)); if (\Scrutinizer\PhpAnalyzer\PhpParser\NodeUtil::isAssignmentOp($node->getAttribute('parent'))) { if ($objType->hasMethod('__set')) { $this->phpFile->addComment($node->getLine(), Comment::warning('strict.maybe_undocument_property_write_capability', 'The property ``%property_name%`` does not exist. Since you implemented ``__set``, maybe consider adding a [@property annotation](http://www.phpdoc.org/docs/latest/for-users/tags/property.html).', array('property_name' => $node->name))); return; } } else { if ($objType->hasMethod('__get')) { $this->phpFile->addComment($node->getLine(), Comment::warning('strict.maybe_undocument_property_read_capability', 'The property ``%property_name%`` does not exist. Since you implemented ``__get``, maybe consider adding a [@property annotation](http://www.phpdoc.org/docs/latest/for-users/tags/property.html).', array('property_name' => $node->name))); return; } } $thisType = $this->t->getScope()->getTypeOfThis(); if ($thisType && $thisType->equals($objType)) { $this->phpFile->addComment($node->getLine(), Comment::warning('strict.undeclared_property', 'The property "%property_name%" does not exist. Did you maybe forget to declare it?', array('property_name' => $node->name))); } else { $this->phpFile->addComment($node->getLine(), Comment::error('typos.non_existent_property', 'The property "%property_name%" does not exist.', array('property_name' => $node->name))); } } }
public function visit(NodeTraversal $t, \PHPParser_Node $node, \PHPParser_Node $parent = null) { // Consider only non-dynamic variables in this pass. if (!$node instanceof \PHPParser_Node_Expr_Variable || !is_string($node->name)) { return; } // We ignore PHP globals. if (NodeUtil::isSuperGlobal($node)) { return; } // Ignore variables which are defined here. if ($parent instanceof \PHPParser_Node_Expr_Assign && $parent->var === $node) { return; } if (!$t->getScope()->isDeclared($node->name)) { if ($bestName = CheckForTyposPass::getMostSimilarName($node->name, $t->getScope()->getVarNames())) { $this->phpFile->addComment($node->getLine(), Comment::error('typos.mispelled_variable_name', 'The variable ``%offending_variable_name%`` does not exist. Did you mean ``%closest_variable_name%``?', array('offending_variable_name' => '$' . $node->name, 'closest_variable_name' => '$' . $bestName))); } else { $this->phpFile->addComment($node->getLine(), Comment::warning('usage.undeclared_variable', 'The variable ``%variable_name%`` does not exist. Did you forget to declare it?', array('variable_name' => '$' . $node->name))); } return; } if ($parent instanceof \PHPParser_Node_Expr_ArrayDimFetch && $node->getAttribute('array_initializing_variable') && $parent->var === $node) { $this->phpFile->addComment($node->getLine(), Comment::warning('usage.non_initialized_array', '``%array_fetch_expr%`` was never initialized. Although not strictly required by PHP, it is generally a good practice to add ``%array_fetch_expr% = array();`` before regardless.', array('array_fetch_expr' => self::$prettyPrinter->prettyPrintExpr($node)))); } }
private function createInAccessiblePropertyError($propertyName, $className, $visibility) { return Comment::error('access_control.inaccessible_property', 'The property "%property_name%" cannot be accessed from this context as it is declared %visibility% in class "%class_name%"?', array('property_name' => $propertyName, 'class_name' => $className, 'visibility' => $visibility)); }
private function checkParameters(\PHPParser_Node $node, AbstractFunction $function, array $args, \Scrutinizer\PhpAnalyzer\Model\MethodContainer $clazz = null) { $missingArgs = $this->argumentChecker->getMissingArguments($function, $args, $clazz); if (!empty($missingArgs)) { if (count($missingArgs) > 1) { $this->phpFile->addComment($node->getLine(), Comment::error('usage.missing_multiple_required_arguments', 'The call to ``%function_name%()`` misses some required arguments starting with ``$%parameter_name%``.', array('function_name' => $function->getName(), 'parameter_name' => reset($missingArgs)))); } else { $this->phpFile->addComment($node->getLine(), Comment::error('usage.missing_required_argument', 'The call to ``%function_name%()`` misses a required argument ``$%parameter_name%``.', array('function_name' => $function->getName(), 'parameter_name' => reset($missingArgs)))); } } if ('disabled' !== $this->getSetting('argument_type_checks')) { $argTypes = array(); foreach ($args as $arg) { $argTypes[] = $arg->getAttribute('type') ?: $this->typeRegistry->getNativeType('unknown'); } $mismatchedTypes = $this->argumentChecker->getMismatchedArgumentTypes($function, $argTypes, $clazz); foreach ($mismatchedTypes as $index => $type) { $this->phpFile->addComment($args[$index]->getLine(), Comment::warning('usage.argument_type_mismatch', '``%expr%`` of type ``%expr_type%`` is not a sub-type of ``%expected_type%``.', array('expr' => self::$prettyPrinter->prettyPrintExpr($args[$index]->value), 'expr_type' => (string) $argTypes[$index], 'expected_type' => (string) $type))); } } }