Skip to content
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

Warning for negative string offset (< PHP 7.1) #1791

Merged
merged 9 commits into from Jun 16, 2018
1 change: 1 addition & 0 deletions .gitignore
Expand Up @@ -15,3 +15,4 @@ all_output.*
.phan/config.local.php
nohup.out
*.md.new
.idea/
41 changes: 33 additions & 8 deletions src/Phan/AST/UnionTypeVisitor.php
Expand Up @@ -1193,6 +1193,7 @@ public function visitDim(Node $node) : UnionType
static $string_type;
static $int_union_type;
static $int_or_string_union_type;

if ($array_access_type === null) {
// array offsets work on strings, unfortunately
// Double check that any classes in the type don't
Expand All @@ -1206,12 +1207,14 @@ public function visitDim(Node $node) : UnionType
$int_union_type = IntType::instance(false)->asUnionType();
$int_or_string_union_type = new UnionType([IntType::instance(false), StringType::instance(false)], true);
}

if ($union_type->hasTopLevelArrayShapeTypeInstances()) {
$element_type = $this->resolveArrayShapeElementTypes($node, $union_type);
if ($element_type !== null) {
return $element_type;
}
}

$dim_type = self::unionTypeFromNode(
$this->code_base,
$this->context,
Expand All @@ -1221,12 +1224,11 @@ public function visitDim(Node $node) : UnionType

// Figure out what the types of accessed array
// elements would be.
$generic_types =
$union_type->genericArrayElementTypes();
$generic_types = $union_type->genericArrayElementTypes();

// If we have generics, we're all set
if (!$generic_types->isEmpty()) {
if ($this->isSuspiciousNullable($union_type) && !($node->flags & self::FLAG_IGNORE_NULLABLE)) {
if (!($node->flags & self::FLAG_IGNORE_NULLABLE) && $this->isSuspiciousNullable($union_type)) {
$this->emitIssue(
Issue::TypeArraySuspiciousNullable,
$node->lineno ?? 0,
Expand All @@ -1235,7 +1237,8 @@ public function visitDim(Node $node) : UnionType
}

if (!$dim_type->isEmpty()) {
if (!$union_type->asExpandedTypes($this->code_base)->hasArrayAccess() && !$union_type->hasMixedType()) {
if (!$union_type->hasMixedType() && !$union_type->asExpandedTypes($this->code_base)->hasArrayAccess()) {

if (Config::getValue('scalar_array_key_cast')) {
$expected_key_type = $int_or_string_union_type;
} else {
Expand All @@ -1244,14 +1247,14 @@ public function visitDim(Node $node) : UnionType
GenericArrayType::CONVERT_KEY_MIXED_TO_INT_OR_STRING_UNION_TYPE
);
}

if (!$dim_type->canCastToUnionType($expected_key_type)) {
$issue_type = Issue::TypeMismatchDimFetch;

if ($dim_type->containsNullable()) {
if ($dim_type->nonNullableClone()->canCastToUnionType($expected_key_type)) {
$issue_type = Issue::TypeMismatchDimFetchNullable;
}
if ($dim_type->containsNullable() && $dim_type->nonNullableClone()->canCastToUnionType($expected_key_type)) {
$issue_type = Issue::TypeMismatchDimFetchNullable;
}

if ($this->should_catch_issue_exception) {
$this->emitIssue(
$issue_type,
Expand All @@ -1262,6 +1265,7 @@ public function visitDim(Node $node) : UnionType
);
return $generic_types;
}

throw new IssueException(
Issue::fromType($issue_type)(
$this->context->getFile(),
Expand Down Expand Up @@ -1289,6 +1293,10 @@ public function visitDim(Node $node) : UnionType
if ($union_type->isType($string_type)
|| ($union_type->canCastToUnionType($string_type->asUnionType()) && !$union_type->hasMixedType())
) {
if (Config::get_closest_target_php_version_id() < 70100 && $union_type->isNonNullStringType()) {
$this->analyzeNegativeStringOffsetCompatibility($node);
}

if (!$dim_type->isEmpty() && !$dim_type->canCastToUnionType($int_union_type)) {
// TODO: Efficient implementation of asExpandedTypes()->hasArrayAccess()?
if (!$union_type->isEmpty() && !$union_type->asExpandedTypes($this->code_base)->hasArrayLike()) {
Expand Down Expand Up @@ -2817,4 +2825,21 @@ public static function anyStringLiteralForNode(
}
return null;
}

/**
* @param Node $node
*/
private function analyzeNegativeStringOffsetCompatibility(Node $node)
{
if (($node->children['dim'] ?? null) instanceof Node
&& $node->children['dim']->kind === \ast\AST_UNARY_OP
&& $node->children['dim']->flags === \ast\flags\UNARY_MINUS
&& \is_int($node->children['dim']->children['expr'] ?? null)
) {
$this->emitIssue(
Issue::CompatibleNegativeStringOffset,
$node->lineno ?? 0
);
}
}
}
9 changes: 7 additions & 2 deletions src/Phan/CLI.php
Expand Up @@ -1053,11 +1053,16 @@ private function maybeReadConfigFile(bool $require_config_exists)
]);

// Totally cool if the file isn't there
if (!file_exists($config_file_name)) {
if ($config_file_name === false || !file_exists($config_file_name)) {
if ($require_config_exists) {
// But if the CLI option --require-config-exists is provided, exit immediately.
// (Include extended help documenting that option)
$this->usage("Could not find a config file at '$config_file_name', but --require-config-exists was set", EXIT_FAILURE, true);
if ($config_file_name !== false) {
$this->usage("Could not find a config file at '$config_file_name', but --require-config-exists was set", EXIT_FAILURE, true);
} else {
$this->usage(sprintf("Could not figure out the path for config file '%s', but --require-config-exists was set", $this->config_file), EXIT_FAILURE, true);
}

}
return;
}
Expand Down
9 changes: 9 additions & 0 deletions src/Phan/Issue.php
Expand Up @@ -311,6 +311,7 @@ class Issue
const CompatibleUseIterablePHP71 = 'PhanCompatibleUseIterablePHP71';
const CompatibleUseObjectPHP71 = 'PhanCompatibleUseObjectPHP71';
const CompatibleMultiExceptionCatchPHP70 = 'PhanCompatibleMultiExceptionCatchPHP70';
const CompatibleNegativeStringOffset = 'PhanCompatibleNegativeStringOffset';

// Issue::CATEGORY_GENERIC
const TemplateTypeConstant = 'PhanTemplateTypeConstant';
Expand Down Expand Up @@ -2614,6 +2615,14 @@ public static function issueMap()
self::REMEDIATION_B,
3011
),
new Issue(
self::CompatibleNegativeStringOffset,
self::CATEGORY_COMPATIBLE,
self::SEVERITY_CRITICAL,
"Using negative string offset is not available before PHP 7.1 (emits an 'Uninitialized string offset' notice)",
self::REMEDIATION_B,
3012
),

// Issue::CATEGORY_GENERIC
new Issue(
Expand Down
1 change: 1 addition & 0 deletions src/Phan/Parse/ParseVisitor.php
Expand Up @@ -1258,4 +1258,5 @@ public function visitBinaryOp(Node $node)
{
return $this->context;
}

}
@@ -0,0 +1,2 @@
src/009_negative_string_offset.php:5 PhanCompatibleNegativeStringOffset Using negative string offset is not available before PHP 7.1 (emits an 'Uninitialized string offset' notice)
src/009_negative_string_offset.php:6 PhanCompatibleNegativeStringOffset Using negative string offset is not available before PHP 7.1 (emits an 'Uninitialized string offset' notice)
9 changes: 9 additions & 0 deletions tests/php70_test/src/009_negative_string_offset.php
@@ -0,0 +1,9 @@
<?php

$string = "Phan is awesome";

$char1 = $string{-1};
$char2 = $string[-2];

// accessing negative array index should not emit the warning
$array_element = [ -1 => 1 ][-1];
5 changes: 4 additions & 1 deletion tests/php70_test/test.sh
Expand Up @@ -12,7 +12,10 @@ if [[ $? != 0 ]]; then
exit 1
fi
echo "Running phan in '$PWD' ..."
rm $ACTUAL_PATH -f || exit 1

if [ -f $ACTUAL_PATH ]; then
rm -f $ACTUAL_PATH || exit 1
fi

# We use the polyfill parser because it behaves consistently in all php versions.
../../phan --force-polyfill-parser --memory-limit 1G | tee $ACTUAL_PATH
Expand Down