Skip to content

implemented DuplicateDeclarationRule, fixes phpstan/phpstan#3475#250

Closed
dktapps wants to merge 10 commits intophpstan:masterfrom
pmmp:duplicate-declarations-rule
Closed

implemented DuplicateDeclarationRule, fixes phpstan/phpstan#3475#250
dktapps wants to merge 10 commits intophpstan:masterfrom
pmmp:duplicate-declarations-rule

Conversation

@dktapps
Copy link
Copy Markdown
Contributor

@dktapps dktapps commented Jun 18, 2020

No description provided.

ondrejmirtes
ondrejmirtes requested changes Jun 18, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

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

This is mostly fine, thank you :)

tests/PHPStan/Rules/Classes/DuplicateDeclarationRuleTest.php
$this->analyse(
[
__DIR__ . '/data/duplicate-declarations.php',
],
Copy link
Copy Markdown
Member

@ondrejmirtes ondrejmirtes Jun 18, 2020

Choose a reason for hiding this comment

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

the indentation looks wrong

Copy link
Copy Markdown
Contributor Author

@dktapps dktapps Jun 18, 2020

Choose a reason for hiding this comment

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

are you sure? it's the same as this one locally: https://github.com/phpstan/phpstan-src/blob/master/tests/PHPStan/Rules/Classes/ClassConstantRuleTest.php

Copy link
Copy Markdown
Member

@ondrejmirtes ondrejmirtes Jun 18, 2020

Choose a reason for hiding this comment

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

Oh sorry, you're right. Anyway, look at the failing build. The files can be excluded from lint like this

phpstan-src/build.xml

Lines 118 to 150 in 6425cdf

<target name="lint">
<exec
executable="vendor/bin/parallel-lint"
logoutput="true"
passthru="true"
checkreturn="true"
>
<arg value="--colors"/>
<arg value="--exclude"/>
<arg path="tests/PHPStan/Analyser/data"/>
<arg value="--exclude"/>
<arg path="tests/PHPStan/Rules/Methods/data"/>
<arg value="--exclude"/>
<arg path="tests/PHPStan/Rules/Functions/data"/>
<arg value="--exclude"/>
<arg path="tests/PHPStan/Rules/Operators/data/invalid-inc-dec.php"/>
<arg value="--exclude"/>
<arg path="tests/PHPStan/Rules/Arrays/data/offset-access-without-dim-for-reading.php"/>
<arg value="--exclude"/>
<arg path="tests/PHPStan/Rules/Classes/data/extends-error.php"/>
<arg value="--exclude"/>
<arg path="tests/PHPStan/Rules/Classes/data/implements-error.php"/>
<arg value="--exclude"/>
<arg path="tests/PHPStan/Rules/Classes/data/interface-extends-error.php"/>
<arg value="--exclude"/>
<arg path="tests/PHPStan/Rules/Classes/data/trait-use-error.php"/>
<arg value="--exclude"/>
<arg path="tests/PHPStan/Rules/Properties/data/default-value-for-native-property-type.php"/>
<arg path="src" />
<arg path="tests" />
<arg path="compiler/src" />
</exec>
</target>

Copy link
Copy Markdown
Contributor Author

@dktapps dktapps Jun 18, 2020

Choose a reason for hiding this comment

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

Thanks. I noticed it had failed but I wasn't sure what to do about it.

@ondrejmirtes
Copy link
Copy Markdown
Member

ondrejmirtes commented Jun 18, 2020

Skip the tests in case self::$useStaticReflectionProvider is false. There are many tests skipped like this.

@ondrejmirtes ondrejmirtes mentioned this pull request Jun 20, 2020
Closed
@ondrejmirtes
Copy link
Copy Markdown
Member

ondrejmirtes commented Jun 20, 2020

Merged as 5e2ba1a, thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ondrejmirtes ondrejmirtes ondrejmirtes requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@dktapps @ondrejmirtes