Skip to content

Conversation

@dereuromark
Copy link
Member

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
*
* @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
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member Author

@dereuromark dereuromark Nov 21, 2017

Choose a reason for hiding this comment

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

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
Copy link

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
@dereuromark dereuromark deleted the master-phpstan branch November 21, 2017 20:01
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