Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Don't suggest private methods #682

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

Don't suggest private methods #682

wants to merge 17 commits into from

Conversation

gnoe
Copy link

@gnoe gnoe commented Nov 10, 2018 •
edited by felixfbecker
Loading

To avoid errors on Travis with line ending.

This branch has the same fix for showing private and protected methods and properties out of the context.

Fixes #469

@felixfbecker
Copy link
Owner

Could you add a test for this?

@gnoe
Copy link
Author

gnoe commented Nov 10, 2018

Yes, I can.
Should I add it to tests\Server\TextDocument\CompletionTest.php?
I tested this launching the php-intellisense extension pointing to my local copy of the language server.
I will write the test.

Travis keeps failing because of the line ending.

@felixfbecker
Copy link
Owner

What editor are you using? You should configure it to trim trailing whitespace (or delete it manually)

@gnoe
Copy link
Author

gnoe commented Nov 10, 2018

I use VS Code and Netbeans, I configured VS Code.
Thanks!
Next commit with the test should be fixed

gnoe added 2 commits November 10, 2018 17:44
@gnoe
Trimmed Trailing Whitespace
e71e4e0
@gnoe
Visibility Test and Changing line ending from CRLF -> LF
73ab152 
Added CompletionWithVisibilityTest.php
- checks visibility of a property or method call (only public)
- checks visibility of a Class (can see all methods and properties)
- checks visibility of a child class (public and protected)
@codecov
Copy link

codecov bot commented Nov 10, 2018 •
edited by ghost
Loading

Codecov Report

Merging #682 into master will increase coverage by 0.2%.
The diff coverage is 93.33%.

@@             Coverage Diff             @@
##             master     #682     +/-   ##
===========================================
+ Coverage     82.17%   82.38%   +0.2%     
- Complexity      935      944      +9     
===========================================
  Files            44       44             
  Lines          2154     2168     +14     
===========================================
+ Hits           1770     1786     +16     
+ Misses          384      382      -2
Impacted Files Coverage Δ Complexity Δ
src/CompletionProvider.php 94.32% <100%> (+0.07%) 113 <0> (+1) ⬆️
src/Definition.php 95.65% <90.9%> (-4.35%) 14 <8> (+8)
src/DefinitionResolver.php 87.8% <0%> (+0.34%) 332% <0%> (ø) ⬇️
src/Index/Index.php 78.22% <0%> (+0.8%) 54% <0%> (ø) ⬇️

gnoe added 4 commits November 11, 2018 00:19
@gnoe
Fixing phpcs errors
fd9d326
@gnoe
CompletionTest fixed
8d4fe1e
@gnoe
Decided to move visibility files apart
9a75adf 
Other tests were failing because I modifed the global_symbols.php file
@gnoe
EOL
fabe20b
@felixfbecker felixfbecker changed the title Fix/eol Don't suggest private methods Nov 11, 2018
@felixfbecker
Copy link
Owner

Would you mind fixing merge conflicts?

@felixfbecker felixfbecker mentioned this pull request Nov 11, 2018
Closed
@gnoe
Copy link
Author

gnoe commented Nov 11, 2018

I did merge the branch already, sorry, I have to get used to the code. I removed the part of the static checking. I would like to test it better.

@codecov-io
Copy link

codecov-io commented Jan 17, 2020 •
edited by ghost
Loading

Codecov Report

Merging #682 into master will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #682   +/-   ##
=========================================
  Coverage     82.38%   82.38%           
  Complexity      944      944           
=========================================
  Files            44       44           
  Lines          2168     2168           
=========================================
  Hits           1786     1786           
  Misses          382      382

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
work needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Intellisense autocomplete also suggests private or protected method and properties
3 participants
@gnoe @felixfbecker @codecov-io