Skip to content

Commit

Permalink
Show code snippets when reporting errors
Browse files Browse the repository at this point in the history 
This also introduces a new method of identifying specific code locations when creating issues
  • Loading branch information
@muglug
muglug committed Dec 4, 2016
1 parent 1d603b1 commit a1acbfe
Show file tree
Hide file tree
Showing 28 changed files with 640 additions and 514 deletions.
22 changes: 13 additions & 9 deletions examples/StringChecker.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,16 @@ class StringChecker extends \Psalm\Plugin
* checks an expression
* @param PhpParser\Node\Expr $stmt
* @param Context $context
* @param string $file_name
* @param CodeLocation $file_name
* @param array<string> $suppressed_issues
* @return null|false
*/
public function checkExpression(PhpParser\Node\Expr $stmt, \Psalm\Context $context, $file_name, array $suppressed_issues)
{
public function checkExpression(
PhpParser\Node\Expr $stmt,
Context $context,
CodeLocation $code_location,
array $suppressed_issues
) {
if ($stmt instanceof \PhpParser\Node\Scalar\String_) {
$class_or_class_method = '/^\\\?Psalm(\\\[A-Z][A-Za-z0-9]+)+(::[A-Za-z0-9]+)?$/';

Expand All @@ -29,19 +33,19 @@ public function checkExpression(PhpParser\Node\Expr $stmt, \Psalm\Context $conte

if (Checker\ClassChecker::checkFullyQualifiedClassLikeName(
$fq_class_name,
$file_name,
$stmt->getLine(),
$suppressed_issues) === false
$code_location,
$suppressed_issues
) === false
) {
return false;
}

if ($fq_class_name !== $stmt->value) {
if (Checker\MethodChecker::checkMethodExists(
$stmt->value,
$file_name,
$stmt->getLine(),
$suppressed_issues)
$code_location,
$suppressed_issues
)
) {
return false;
}
Expand Down
51 changes: 27 additions & 24 deletions src/Psalm/Checker/ClassLikeChecker.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
namespace Psalm\Checker;

use PhpParser;
use Psalm\CodeLocation;
use Psalm\Config;
use Psalm\Context;
use Psalm\Exception\DocblockParseException;
Expand Down Expand Up @@ -202,7 +203,9 @@ public function __construct(PhpParser\Node\Stmt\ClassLike $class, StatementsSour
$this->aliased_constants = $source->getAliasedConstants();
$this->aliased_functions = $source->getAliasedFunctions();
$this->file_name = $source->getFileName();
$this->file_path = $source->getFilePath();
$this->include_file_name = $source->getIncludeFileName();
$this->include_file_path = $source->getIncludeFilePath();
$this->fq_class_name = $fq_class_name;

$this->suppressed_issues = $source->getSuppressedIssues();
Expand Down Expand Up @@ -289,8 +292,7 @@ public function check($check_methods = true, Context $class_context = null, $upd
foreach ($parent_interfaces as $interface_name) {
if (self::checkFullyQualifiedClassLikeName(
$interface_name,
$this->file_name,
$this->class->getLine(),
new CodeLocation($this, $this->class, true),
$this->getSuppressedIssues()
) === false) {
return false;
Expand Down Expand Up @@ -392,8 +394,7 @@ public function check($check_methods = true, Context $class_context = null, $upd
if (IssueBuffer::accepts(
new UnimplementedInterfaceMethod(
'Method ' . $method_name . ' is not defined on class ' . $this->fq_class_name,
$this->file_name,
$this->class->getLine()
new CodeLocation($this, $this->class)
),
$this->suppressed_issues
)) {
Expand Down Expand Up @@ -434,10 +435,17 @@ public function check($check_methods = true, Context $class_context = null, $upd
*/
protected function registerParentClassProperties($parent_class)
{
if (!$this->class instanceof PhpParser\Node\Stmt\Class_) {
throw new \UnexpectedValueException('Cannot register parent class where none exists');
}

if (!$this->class->extends) {
throw new \UnexpectedValueException('Cannot register parent class where none exists');
}

if (self::checkFullyQualifiedClassLikeName(
$parent_class,
$this->file_name,
$this->class->getLine(),
new CodeLocation($this, $this->class->extends, true),
$this->getSuppressedIssues()
) === false
) {
Expand Down Expand Up @@ -535,7 +543,10 @@ protected function visitTraitUse(

if (!TraitChecker::traitExists($trait_name)) {
if (IssueBuffer::accepts(
new UndefinedTrait('Trait ' . $trait_name . ' does not exist', $this->file_name, $trait->getLine()),
new UndefinedTrait(
'Trait ' . $trait_name . ' does not exist',
new CodeLocation($this, $trait)
),
$this->suppressed_issues
)) {
return false;
Expand All @@ -547,8 +558,7 @@ protected function visitTraitUse(
if (IssueBuffer::accepts(
new UndefinedTrait(
'Trait ' . $trait_name . ' has wrong casing',
$this->file_name,
$trait->getLine()
new CodeLocation($this, $trait)
),
$this->suppressed_issues
)) {
Expand Down Expand Up @@ -596,13 +606,11 @@ protected function visitPropertyDeclaration(
if ($comment && $config->use_docblock_types) {
try {
$type_in_comment = CommentChecker::getTypeFromComment((string) $comment, null, $this);
}
catch (DocblockParseException $e) {
} catch (DocblockParseException $e) {
if (IssueBuffer::accepts(
new InvalidDocblock(
(string)$e->getMessage(),
$this->getCheckedFileName(),
$stmt->getLine()
new CodeLocation($this, $this->class, true)
)
)) {
return false;
Expand All @@ -613,8 +621,7 @@ protected function visitPropertyDeclaration(
new MissingPropertyType(
'Property ' . $this->fq_class_name . '::$' . $stmt->props[0]->name . ' does not have a ' .
'declared type',
$this->file_name,
$stmt->getLine()
new CodeLocation($this, $stmt)
),
$this->suppressed_issues
)) {
Expand Down Expand Up @@ -757,15 +764,13 @@ public static function classExtendsOrImplements($fq_class_name, $possible_parent

/**
* @param string $fq_class_name
* @param string $file_name
* @param int $line_number
* @param CodeLocation $code_location
* @param array<string> $suppressed_issues
* @return bool|null
*/
public static function checkFullyQualifiedClassLikeName(
$fq_class_name,
$file_name,
$line_number,
CodeLocation $code_location,
array $suppressed_issues
) {
if (empty($fq_class_name)) {
Expand All @@ -781,8 +786,7 @@ public static function checkFullyQualifiedClassLikeName(
if (IssueBuffer::accepts(
new UndefinedClass(
'Class or interface ' . $fq_class_name . ' does not exist',
$file_name,
$line_number
$code_location
),
$suppressed_issues
)) {
Expand All @@ -798,16 +802,15 @@ public static function checkFullyQualifiedClassLikeName(
if (IssueBuffer::accepts(
new InvalidClass(
'Class or interface ' . $fq_class_name . ' has wrong casing',
$file_name,
$line_number
$code_location
),
$suppressed_issues
)) {
return false;
}
}

FileChecker::addFileReferenceToClass(Config::getInstance()->getBaseDir() . $file_name, $fq_class_name);
FileChecker::addFileReferenceToClass(Config::getInstance()->getBaseDir() . $code_location->file_name, $fq_class_name);

return true;
}
Expand Down
47 changes: 33 additions & 14 deletions src/Psalm/Checker/CommentChecker.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,23 +78,25 @@ public static function getTypeFromComment(
}

/**
* @param string $comment
* @param string $comment
* @param int $line_number
* @return FunctionDocblockComment
* @throws DocblockParseException If there was a problem parsing the docblock.
* @psalm-suppress MixedArrayAccess
*/
public static function extractDocblockInfo($comment)
public static function extractDocblockInfo($comment, $line_number)
{
$comments = self::parseDocComment($comment);
$comments = self::parseDocComment($comment, $line_number);

$info = new FunctionDocblockComment();

if (isset($comments['specials']['return']) || isset($comments['specials']['psalm-return'])) {
$return_block = trim(
isset($comments['specials']['psalm-return'])
? (string)$comments['specials']['psalm-return'][0]
: (string)$comments['specials']['return'][0]
);
/** @var array */
$return_specials = isset($comments['specials']['psalm-return'])
? $comments['specials']['psalm-return']
: $comments['specials']['return'];

$return_block = trim((string)reset($return_specials));

try {
$line_parts = self::splitDocLine($return_block);
Expand All @@ -107,11 +109,12 @@ public static function extractDocblockInfo($comment)
&& !strpos($line_parts[0], '::')
) {
$info->return_type = $line_parts[0];
$info->return_type_line_number = array_keys($return_specials)[0];
}
}

if (isset($comments['specials']['param'])) {
foreach ($comments['specials']['param'] as $param) {
foreach ($comments['specials']['param'] as $line_number => $param) {
try {
$line_parts = self::splitDocLine((string)$param);
} catch (DocblockParseException $e) {
Expand All @@ -128,7 +131,11 @@ public static function extractDocblockInfo($comment)
$line_parts[1] = substr($line_parts[1], 1);
}

$info->params[] = ['name' => substr($line_parts[1], 1), 'type' => $line_parts[0]];
$info->params[] = [
'name' => substr($line_parts[1], 1),
'type' => $line_parts[0],
'line_number' => $line_number
];
}
}
}
Expand Down Expand Up @@ -202,9 +209,10 @@ protected static function splitDocLine($return_block)
* https://github.com/facebook/libphutil/blob/master/src/parser/docblock/PhutilDocblockParser.php
*
* @param string $docblock
* @param int $line_number
* @return array Array of the main comment and specials
*/
public static function parseDocComment($docblock)
public static function parseDocComment($docblock, $line_number = null)
{
// Strip off comments.
$docblock = trim($docblock);
Expand All @@ -214,6 +222,13 @@ public static function parseDocComment($docblock)

// Normalize multi-line @specials.
$lines = explode("\n", $docblock);

$line_map = [];

if ($line_number) {
$line_number++;
}

$last = false;
foreach ($lines as $k => $line) {
if (preg_match('/^\s?@\w/i', $line)) {
Expand All @@ -224,6 +239,10 @@ public static function parseDocComment($docblock)
$lines[$last] = rtrim($lines[$last]).' '.trim($line);
unset($lines[$k]);
}

if ($line_number) {
$line_map[$line] = $line_number++;
}
}

$docblock = implode("\n", $lines);
Expand All @@ -235,14 +254,14 @@ public static function parseDocComment($docblock)
$have_specials = preg_match_all('/^\s?@([\w\-:]+)\s*([^\n]*)/m', $docblock, $matches, PREG_SET_ORDER);
if ($have_specials) {
$docblock = preg_replace('/^\s?@([\w\-:]+)\s*([^\n]*)/m', '', $docblock);
foreach ($matches as $match) {
foreach ($matches as $m => $match) {
list($_, $type, $data) = $match;

if (empty($special[$type])) {
$special[$type] = array();
$special[$type] = [];
}

$special[$type][] = $data;
$special[$type][$line_map && isset($line_map[$_]) ? $line_map[$_] : $m] = $data;
}
}

Expand Down
Loading

0 comments on commit a1acbfe

Please sign in to comment.