Skip to content

Create phpcs static check for DiConfigTest #233

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

Merged
merged 12 commits into from
Aug 30, 2021

Conversation

loginesta
Copy link
Contributor

@loginesta loginesta commented Aug 25, 2021

This sniffer looks for occurrences of obsolete nodes: param, instance, array, item[@key] and value.
Returns a warning for each occurrence found.
https://jira.corp.magento.com/browse/AC-660

- DiConfigTest: Test for obsolete nodes in di.xml
@sivaschenko sivaschenko changed the title AC-660: Create phpcs static check for DiConfigTest Create phpcs static check for DiConfigTest Aug 25, 2021
Copy link
Member

@sivaschenko sivaschenko left a comment

Choose a reason for hiding this comment

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

Thanks for the pull request @loginesta ! Can you please verify the test functionality running it against the Magento code with changed di.xml files?

@loginesta loginesta marked this pull request as ready for review August 27, 2021 14:31
Copy link
Member

@sivaschenko sivaschenko left a comment

Choose a reason for hiding this comment

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

Thanks for the updates @loginesta ! Please see my comments

{
private const WARNING_CODE = 'FoundObsoleteAttribute';

private $xpathObsoleteElems = [
Copy link
Member

@sivaschenko sivaschenko Aug 27, 2021

Choose a reason for hiding this comment

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

I would renamve this property as it's not xpath anymore, also it would be good to add a phpdoc

Suggested change
private $xpathObsoleteElems = [
private $obsoleteDiNodes = [

{
$lineContent = $phpcsFile->getTokensAsString($stackPtr, 1);

foreach ($this->xpathObsoleteElems as $elem => $message) {
Copy link
Member

Choose a reason for hiding this comment

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

It's a good pratice to use whole words for variables naming

Suggested change
foreach ($this->xpathObsoleteElems as $elem => $message) {
foreach ($this->xpathObsoleteElems as $element => $message) {

- Refactor: Rename variables and add phpdoc
@sivaschenko
Copy link
Member

@magento import pr to magento-commerce/magento-coding-standard

@magento-engcom-team
Copy link
Contributor

@sivaschenko the pull request successfully imported.

@magento-devops-reposync-svc magento-devops-reposync-svc merged commit 20b1378 into magento:develop Aug 30, 2021
@loginesta loginesta deleted the AC-660 branch September 1, 2021 08:27
magento-devops-reposync-svc pushed a commit that referenced this pull request Aug 9, 2023
…o-coding-standard-453

[Imported] Update supported PHP versions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants