Skip to content
This repository has been archived by the owner on Jul 26, 2021. It is now read-only.

Fix getVisibility() for methods without keyword #97

Closed
wants to merge 1 commit into from
Closed

Fix getVisibility() for methods without keyword #97

wants to merge 1 commit into from

Conversation

villfa
Copy link

@villfa villfa commented Jul 18, 2020

Methods declared without any explicit visibility keyword are defined as public

Methods declared without any explicit visibility keyword are defined as public
@codecov
Copy link

codecov bot commented Jul 18, 2020

Codecov Report

Merging #97 into master will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #97      +/-   ##
============================================
+ Coverage     82.13%   82.17%   +0.03%     
  Complexity      228      228              
============================================
  Files            11       11              
  Lines           515      516       +1     
============================================
+ Hits            423      424       +1     
  Misses           92       92              
Impacted Files Coverage Δ Complexity Δ
src/TokenWithScopeAndVisibility.php 100.00% <100.00%> (ø) 20.00 <0.00> (ø)

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 152aa3e...e1e74d7. Read the comment docs.

Copy link

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

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

Is it just me or is there no test without visibility declared? In which case, this change would still not be tested.

@villfa
Copy link
Author

villfa commented Jul 18, 2020

@jrfnl it is just you ;)
See here:

class Foo{function foo(){}

@jrfnl
Copy link

jrfnl commented Jul 18, 2020

@villfa Glad to hear it. I completely missed that test case as it is on the same line as the class declaration 🤦

@sebastianbergmann
Copy link
Owner

Thank you for your contribution. I appreciate the time you invested in preparing this pull request. I have yet to decide what happens to this component once I have finished working on sebastianbergmann/php-code-coverage#777. I will only merge this pull request if I decide to maintain this component after I no longer need it myself. To be frank, this is unlikely.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants