Skip to content

Fixes #5538#5540

Open
scorgn wants to merge 2 commits into
phan:v6from
scorgn:bug5538
Open

Fixes #5538#5540
scorgn wants to merge 2 commits into
phan:v6from
scorgn:bug5538

Conversation

@scorgn
Copy link
Copy Markdown

@scorgn scorgn commented Jun 5, 2026

FIxes #5538

Problem

When checking for isAssignmentOrNestedAssignmentOrModification within src/Phan/Analysis/PostOrderAnalysisVisitor.php, we only looked for ast\AST_ASSIGN_OP as a top level assignment, but not with nested array assignment (ast\AST_DIM)

Solution

This adds a special check for isNestedNullCoalesceAssignment within isAssignmentOrNestedAssignmentOrModification when handling ast\AST_DIM.


This also creates the tests:

  • tests/plugin_test/src/320_nullish_assignment_operator_private_property.php
  • tests/plugin_test/src/321_addition_concatenation_assignment_operators_private_property.php

* Add `ast\AST_ASSIGN_OP`  to list of assignment operator cases within `shouldSkipNestedAssignDim`
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 118474a5b4

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

break;
case ast\AST_ASSIGN:
case ast\AST_ASSIGN_REF:
case ast\AST_ASSIGN_OP:
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Limit nested-dim skip to coalesce assignments

Treating every AST_ASSIGN_OP as a write-only nested dimension suppresses checks for non-coalescing operators such as $a['missing']['x'] += 1 or .=. Those operators read the left-hand side before writing it, so the intermediate dimension still must be validated; only ??= has the initialization semantics this helper is trying to model. Please gate this case on flags\BINARY_COALESCE so invalid/undeclared nested dimensions are not hidden for ordinary assignment operators.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. I have fixed that and have also added a test explicitly for addition and concatenation assignment.

@scorgn scorgn changed the title Fixes #5538 Draft: Fixes #5538 Jun 5, 2026
* Remove check for  `ast\AST_ASSIGN_OP` inside `shouldSkipNestedAssignDim` since that is used inversely for read & write
* Add check for isNestedNullCoalesceAssignment within isAssignmentOrNestedAssignmentOrModification
* Add tests to ensure issue is still emitted with assignment and concatenation operators
* Move test from tests/files to tests/plugin_test where dead code issues are emitted
@scorgn scorgn changed the title Draft: Fixes #5538 Fixes #5538 Jun 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PhanReadOnlyPrivateProperty emitted when writes exist using the null coalescing assignment operator to set array key

1 participant