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

Fix mixing property and param attributes on promoted property #2825

Closed
wants to merge 1 commit into from

Conversation

RobertMe
Copy link
Contributor

@RobertMe RobertMe commented Dec 13, 2023

Simplify the ParamAttributesRule by merging AttributeCheck::check calls and using an int mask to check for both TARGET_PARAMETER and TARGET_PROPERTY in one pass.

Fixes phpstan/phpstan#10298

Simplify the ParamAttributesRule by merging AttributeCheck::check calls
and using an int mask to check for both TARGET_PARAMETER and
TARGET_PROPERTY in one pass.

Fixes #10298
@ondrejmirtes
Copy link
Member

There is no bug to fix: phpstan/phpstan#10298 (comment)

public function __construct(
#[PropAttr]
#[ParamAttr]
public string $test
Copy link

Choose a reason for hiding this comment

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

Coming from this issue phpstan/phpstan#10385 maybe you could improve the test suite by adding another test case to cover when an attribute has both property and parameter targets like:

#[\Attribute(\Attribute::TARGET_PROPERTY | \Attribute::TARGET_PARAMETER)]
class PropAndParamAttr {}

class Test
{
	public function __construct(
		#[PropAndParamAttr]
		public string $test
	) {}
}

Copy link
Member

Choose a reason for hiding this comment

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

I believe there is already such a test. Feel free to check that and send a PR in case it isn't 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants