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

More phpstan related fixes. #11451

Merged
merged 2 commits into from Nov 21, 2017

Conversation

Projects
None yet
4 participants
@dereuromark
Member

dereuromark commented Nov 21, 2017

PHPSTAN level 3

[ERROR] Found 62 errors    

=>

[ERROR] Found 25 errors       

:-)

We should also start inline deprecations for specific (and really bad) default values like bool where in future API it will actually have to be null (as per strict php7.1+).
This way people can already be a bit more aware and maybe adjust already the code to reflect this.

@dereuromark dereuromark added this to the 3.5.7 milestone Nov 21, 2017

@@ -87,8 +87,8 @@
* When called with a Table argument, the default table object will be set
* and this query object will be returned for chaining.
*
* @param \Cake\Datasource\RepositoryInterface|null $table The default table object to use
* @return \Cake\Datasource\RepositoryInterface|$this
* @param \Cake\Datasource\RepositoryInterface|\Cake\ORM\Table|null $table The default table object to use

This comment has been minimized.

@ADmad

ADmad Nov 21, 2017

Member

The param is type hinted as RepositoryInterface and Table implements that interface, so why is explicitly specifying Table necessary?

@ADmad

ADmad Nov 21, 2017

Member

The param is type hinted as RepositoryInterface and Table implements that interface, so why is explicitly specifying Table necessary?

This comment has been minimized.

@dereuromark

dereuromark Nov 21, 2017

Member

Because it is used in other places with the concrete one (code smell that cant be fixed until the next major).
PS: The IDE shows this also as yellow (mismatch in type).

@dereuromark

dereuromark Nov 21, 2017

Member

Because it is used in other places with the concrete one (code smell that cant be fixed until the next major).
PS: The IDE shows this also as yellow (mismatch in type).

@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Nov 21, 2017

Codecov Report

Merging #11451 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #11451      +/-   ##
============================================
- Coverage     93.12%   93.12%   -0.01%     
  Complexity    13008    13008              
============================================
  Files           436      436              
  Lines         33697    33699       +2     
============================================
+ Hits          31380    31381       +1     
- Misses         2317     2318       +1
Impacted Files Coverage Δ Complexity Δ
src/Controller/Controller.php 95.28% <ø> (ø) 77 <0> (ø) ⬇️
src/Core/functions.php 83.33% <ø> (ø) 0 <0> (ø) ⬇️
src/Filesystem/File.php 88.1% <ø> (ø) 101 <0> (ø) ⬇️
src/Http/Runner.php 100% <ø> (ø) 3 <0> (ø) ⬇️
src/Network/Socket.php 71.03% <ø> (ø) 58 <0> (ø) ⬇️
src/Datasource/QueryTrait.php 98.09% <ø> (ø) 44 <0> (ø) ⬇️
src/View/Helper/FormHelper.php 96.23% <ø> (ø) 368 <0> (ø) ⬇️
src/TestSuite/Fixture/TestFixture.php 89.88% <ø> (ø) 69 <0> (ø) ⬇️
src/I18n/Number.php 97.8% <ø> (ø) 42 <0> (ø) ⬇️
src/Cache/Engine/NullEngine.php 25% <ø> (ø) 12 <0> (ø) ⬇️
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2ff4a9b...afb928c. Read the comment docs.

codecov-io commented Nov 21, 2017

Codecov Report

Merging #11451 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #11451      +/-   ##
============================================
- Coverage     93.12%   93.12%   -0.01%     
  Complexity    13008    13008              
============================================
  Files           436      436              
  Lines         33697    33699       +2     
============================================
+ Hits          31380    31381       +1     
- Misses         2317     2318       +1
Impacted Files Coverage Δ Complexity Δ
src/Controller/Controller.php 95.28% <ø> (ø) 77 <0> (ø) ⬇️
src/Core/functions.php 83.33% <ø> (ø) 0 <0> (ø) ⬇️
src/Filesystem/File.php 88.1% <ø> (ø) 101 <0> (ø) ⬇️
src/Http/Runner.php 100% <ø> (ø) 3 <0> (ø) ⬇️
src/Network/Socket.php 71.03% <ø> (ø) 58 <0> (ø) ⬇️
src/Datasource/QueryTrait.php 98.09% <ø> (ø) 44 <0> (ø) ⬇️
src/View/Helper/FormHelper.php 96.23% <ø> (ø) 368 <0> (ø) ⬇️
src/TestSuite/Fixture/TestFixture.php 89.88% <ø> (ø) 69 <0> (ø) ⬇️
src/I18n/Number.php 97.8% <ø> (ø) 42 <0> (ø) ⬇️
src/Cache/Engine/NullEngine.php 25% <ø> (ø) 12 <0> (ø) ⬇️
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2ff4a9b...afb928c. Read the comment docs.

@markstory markstory merged commit 6d42b5f into master Nov 21, 2017

6 checks passed

codecov/patch 100% of diff hit (target 93.12%)
Details
codecov/project Absolute coverage decreased by -<.01% but relative coverage increased by +6.87% compared to 2ff4a9b
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
stickler-ci No lint errors found.

@dereuromark dereuromark deleted the master-phpstan branch Nov 21, 2017

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