sabre-io / cache

:handbag: Simple cache abstraction layer implementing PSR-16
BSD 3-Clause "New" or "Revised" License
54 stars 10 forks source link

Call static assert functions with self:: #49

Closed phil-davis closed 1 year ago

phil-davis commented 1 year ago

Why have we been writing $this-> rather than self:: when calling the static functions in the Assert library?

codecov[bot] commented 1 year ago

Codecov Report

Merging #49 (b2ad303) into master (485646d) will not change coverage. The diff coverage is n/a.

@@            Coverage Diff            @@
##             master      #49   +/-   ##
=========================================
  Coverage     98.72%   98.72%           
  Complexity       85       85           
=========================================
  Files             4        4           
  Lines           157      157           
=========================================
  Hits            155      155           
  Misses            2        2           

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

phil-davis commented 1 year ago

https://github.com/sebastianbergmann/phpunit/issues/1914

https://phpunit.de/manual/6.5/en/appendixes.assertions.html#appendixes.assertions.static-vs-non-static-usage-of-assertion-methods

Links with some discussion.

phil-davis commented 1 year ago

It seems that some IDEs these days are warning about $this->assertSomething() calls that are actually calling a static function. So maybe that is a reason to change the code?

(it is a very easy global search-replace to make this change)

staabm commented 1 year ago

I think the reason we did not catch it earlier is, that we don't have phpstan/phpunit installed and therefore phpstan didn't report it :)

phil-davis commented 1 year ago

Lots of other projects use $this->assertSomething

@DeepDiver1975 mentioned it in https://github.com/owncloud/core/pull/40600#discussion_r1099882923 just today. But we have been doing it the $this-> way for many years.

DeepDiver1975 commented 1 year ago

My IDE (phpstorm) is crying if we use $this->assert* :rofl: