Skip to content

Introduces AddSensitiveParameterAttributeRector rule#4342

Merged
TomasVotruba merged 3 commits intorectorphp:mainfrom
peterfox:feature/sensitive-parameters
Jun 26, 2023
Merged

Introduces AddSensitiveParameterAttributeRector rule#4342
TomasVotruba merged 3 commits intorectorphp:mainfrom
peterfox:feature/sensitive-parameters

Conversation

@peterfox
Copy link
Copy Markdown
Contributor

Changes

  • Adds the AddSensitiveParameterAttributeRector rule
  • Adds working tests
  • Adds documentation

Why

PHP 8.2 introduced a SensitiveParameter attribute to stop strings from appearing in stack traces. This Rule makes it easy to list the names of parameters you would like the attribute applied upon.

@peterfox peterfox requested a review from TomasVotruba as a code owner June 25, 2023 12:34
Copy link
Copy Markdown
Member

@samsonasik samsonasik left a comment

Choose a reason for hiding this comment

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

I think it should live in Transform namespace since it optional and require configurable, something like ReturnTypeWillChangeRector

https://github.com/rectorphp/rector-src/blob/d50368339398499bb767e80634a4987577c44df5/rules/Transform/Rector/ClassMethod/ReturnTypeWillChangeRector.php#L26C13-L26C39

@TomasVotruba
Copy link
Copy Markdown
Member

@samsonasik That looks like a convention, but looking at the ReturnTypeWillChangeRector, it seems the ReturnTypeWillChangeRector should be part of PHPx set, as bounded strongly to the PHP version. Also, Transform should contains rules that change one node to another, e.g. func call to static call.

At the moment, lets go with PHP category here 👍

@TomasVotruba TomasVotruba merged commit 715561c into rectorphp:main Jun 26, 2023
@TomasVotruba
Copy link
Copy Markdown
Member

Thank you @peterfox for the rule 👍

@peterfox
Copy link
Copy Markdown
Contributor Author

@TomasVotruba thank you for merging it! 👍

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.

4 participants