Skip to content

Commit

Permalink
Emit issues for null/mixed array access
Browse files Browse the repository at this point in the history
  • Loading branch information
muglug committed Nov 22, 2016
1 parent cf0c0cd commit 4f95c67
Show file tree
Hide file tree
Showing 9 changed files with 56 additions and 7 deletions.
2 changes: 2 additions & 0 deletions src/Psalm/Checker/CommentChecker.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ class CommentChecker
* @param string $var_id
* @return Type\Union|null
* @throws DocblockParseException If there was a problem parsing the docblock.
* @psalm-suppress MixedArrayAccess
*/
public static function getTypeFromComment(
$comment,
Expand Down Expand Up @@ -80,6 +81,7 @@ public static function getTypeFromComment(
* @param string $comment
* @return FunctionDocblockComment
* @throws DocblockParseException If there was a problem parsing the docblock.
* @psalm-suppress MixedArrayAccess
*/
public static function extractDocblockInfo($comment)
{
Expand Down
2 changes: 2 additions & 0 deletions src/Psalm/Checker/FileChecker.php
Original file line number Diff line number Diff line change
Expand Up @@ -698,6 +698,7 @@ public static function goodRun($start_time)
$cache_directory .= '/' . self::PARSER_CACHE_DIRECTORY;

if (is_dir($cache_directory)) {
/** @var array<string> */
$directory_files = scandir($cache_directory);

foreach ($directory_files as $directory_file) {
Expand Down Expand Up @@ -743,6 +744,7 @@ public static function deleteOldParserCaches($time_before)
$cache_directory .= '/' . self::PARSER_CACHE_DIRECTORY;

if (is_dir($cache_directory)) {
/** @var array<string> */
$directory_files = scandir($cache_directory);

foreach ($directory_files as $directory_file) {
Expand Down
32 changes: 28 additions & 4 deletions src/Psalm/Checker/Statements/Expression/FetchChecker.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use Psalm\Issue\InvalidPropertyFetch;
use Psalm\Issue\InvisibleProperty;
use Psalm\Issue\MissingPropertyType;
use Psalm\Issue\MixedArrayAccess;
use Psalm\Issue\MixedArrayOffset;
use Psalm\Issue\MixedPropertyFetch;
use Psalm\Issue\NoInterfaceProperties;
Expand Down Expand Up @@ -904,19 +905,42 @@ public static function checkArrayAccess(

$stmt->inferredType = Type::getString();
} elseif ($type->isNull()) {
// @todo emit NullArrayAccess issue
if (IssueBuffer::accepts(
new NullReference(
'Cannot access array value on possibly null variable ' . $array_var_id . ' of type ' . $var_type,
$statements_checker->getCheckedFileName(),
$stmt->getLine()
),
$statements_checker->getSuppressedIssues()
)) {
$stmt->inferredType = Type::getMixed();
break;
}
} elseif ($type->isMixed() || $type->isEmpty()) {
// @todo emit MixedArrayAccess issue
} elseif ($type->value && !ClassChecker::classImplements($type->value, 'ArrayAccess')) {
if (IssueBuffer::accepts(
new MixedArrayAccess(
'Cannot access array value on mixed variable ' . $array_var_id,
$statements_checker->getCheckedFileName(),
$stmt->getLine()
),
$statements_checker->getSuppressedIssues()
)) {
$stmt->inferredType = Type::getMixed();
break;
}
} elseif (strtolower($type->value) !== 'simplexmlelement' &&
!ClassChecker::classImplements($type->value, 'ArrayAccess')
) {
if (IssueBuffer::accepts(
new InvalidArrayAccess(
'Cannot access value on non-array variable ' . $var_id . ' of type ' . $var_type,
'Cannot access array value on non-array variable ' . $array_var_id . ' of type ' . $var_type,
$statements_checker->getCheckedFileName(),
$stmt->getLine()
),
$statements_checker->getSuppressedIssues()
)) {
$stmt->inferredType = Type::getMixed();
break;
}
}
}
Expand Down
7 changes: 7 additions & 0 deletions src/Psalm/Checker/Statements/ExpressionChecker.php
Original file line number Diff line number Diff line change
Expand Up @@ -964,6 +964,13 @@ protected static function fleshOutAtomicType(Type\Atomic $return_type, array $ar
} elseif ($return_type->value[0] === '$' && $method_id) {
$method_params = MethodChecker::getMethodParams($method_id);

if (!$method_params) {
throw new \InvalidArgumentException(
'Cannot get method params of ' . $method_id,
null
);
}

foreach ($args as $i => $arg) {
$method_param = $method_params[$i];

Expand Down
4 changes: 2 additions & 2 deletions src/Psalm/Checker/TypeChecker.php
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,8 @@ public static function getNegatableTypeAssertions(
}

/**
* @param array $left_assertions
* @param array $right_assertions
* @param array<string, string> $left_assertions
* @param array<string, string> $right_assertions
* @return array
*/
private static function combineTypeAssertions(array $left_assertions, array $right_assertions)
Expand Down
4 changes: 4 additions & 0 deletions src/Psalm/Config.php
Original file line number Diff line number Diff line change
Expand Up @@ -177,13 +177,15 @@ public static function loadFromXML($file_name)
}

if (isset($config_xml->mockClasses) && isset($config_xml->mockClasses->class)) {
/** @var \SimpleXMLElement $mock_class */
foreach ($config_xml->mockClasses->class as $mock_class) {
$config->mock_classes[] = $mock_class['name'];
}
}

// this plugin loading system borrows heavily from etsy/phan
if (isset($config_xml->plugins) && isset($config_xml->plugins->plugin)) {
/** @var \SimpleXMLElement $plugin */
foreach ($config_xml->plugins->plugin as $plugin) {
$plugin_file_name = $plugin['filename'];

Expand Down Expand Up @@ -213,6 +215,7 @@ public static function loadFromXML($file_name)
}

if (isset($config_xml->issueHandler)) {
/** @var \SimpleXMLElement $issue_handler */
foreach ($config_xml->issueHandler->children() as $key => $issue_handler) {
if (isset($issue_handler['errorLevel'])) {
$error_level = (string) $issue_handler['errorLevel'];
Expand Down Expand Up @@ -249,6 +252,7 @@ public static function getInstance()
* @param array<\SimpleXMLElement> $extensions
* @return void
* @throws ConfigException If a Config file could not be found.
* @psalm-suppress MixedArrayAccess
*/
protected function loadFileExtensions($extensions)
{
Expand Down
4 changes: 4 additions & 0 deletions src/Psalm/Config/FileFilter.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,25 +63,29 @@ public static function loadFromXML(SimpleXMLElement $e, $inclusive)
$filter->inclusive = true;

if ($e->directory) {
/** @var \SimpleXMLElement $directory */
foreach ($e->directory as $directory) {
$filter->include_dirs[] = self::slashify((string)$directory['name']);
}
}

if ($e->file) {
/** @var \SimpleXMLElement $file */
foreach ($e->file as $file) {
$filter->include_files[] = $file['name'];
$filter->include_files_lowercase[] = strtolower((string)$file['name']);
}
}
} else {
if ($e->directory) {
/** @var \SimpleXMLElement $directory */
foreach ($e->directory as $directory) {
$filter->exclude_dirs[] = self::slashify((string)$directory['name']);
}
}

if ($e->file) {
/** @var \SimpleXMLElement $file */
foreach ($e->file as $file) {
$filter->exclude_files[] = (string)$file['name'];
$filter->exclude_files_lowercase[] = strtolower((string)$file['name']);
Expand Down
6 changes: 6 additions & 0 deletions src/Psalm/Issue/MixedArrayAccess.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<?php
namespace Psalm\Issue;

class MixedArrayAccess extends CodeError
{
}
2 changes: 1 addition & 1 deletion src/Psalm/Type.php
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ public static function convertSquareBrackets($type)
$class_chars = '[a-zA-Z\<\>\\\\_]+';
return preg_replace_callback(
'/(' . $class_chars . '|' . '\((' . $class_chars . '(\|' . $class_chars . ')*' . ')\))((\[\])+)/',
function ($matches) {
function (array $matches) {
$inner_type = str_replace(['(', ')'], '', (string)$matches[1]);

$dimensionality = strlen((string)$matches[4]) / 2;
Expand Down

0 comments on commit 4f95c67

Please sign in to comment.