Skip to content

Fix missing '()' for function definition #340

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

Merged
merged 2 commits into from
Apr 1, 2017
Merged

Fix missing '()' for function definition #340

merged 2 commits into from
Apr 1, 2017

Conversation

Talv
Copy link
Contributor

@Talv Talv commented Apr 1, 2017

Should fix:
#311
#325
#326

Copy link
Contributor

@jens1o jens1o left a comment

Choose a reason for hiding this comment

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

lgtm

@codecov
Copy link

codecov bot commented Apr 1, 2017

Codecov Report

Merging #340 into master will not change coverage.
The diff coverage is 0%.

@@            Coverage Diff            @@
##             master     #340   +/-   ##
=========================================
  Coverage     84.18%   84.18%           
  Complexity      812      812           
=========================================
  Files            59       59           
  Lines          1720     1720           
=========================================
  Hits           1448     1448           
  Misses          272      272
Impacted Files Coverage Δ Complexity Δ
src/DefinitionResolver.php 81.85% <0%> (ø) 276 <0> (ø) ⬇️

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 4d0a0a2...ee43664. Read the comment docs.

@@ -446,7 +446,7 @@ public function resolveExpressionNodeToType(Node\Expr $expr): Type
// Cannot get type for dynamic function call
return new Types\Mixed;
}
$fqn = (string)($expr->getAttribute('namespacedName') ?? $expr->name);
$fqn = (string)($expr->getAttribute('namespacedName') ?? $expr->name . '()');
Copy link
Owner

Choose a reason for hiding this comment

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

Pretty sure namespacedName would need to get the () too, so you have to move the parenthesis

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, you're right. I have made wrong assumption as to what namespacedName actually contains.

@felixfbecker felixfbecker merged commit 14a6d65 into felixfbecker:master Apr 1, 2017
@felixfbecker
Copy link
Owner

Thanks for getting to the bottom of this!

lialan pushed a commit to lambdalab/php-language-server that referenced this pull request Apr 30, 2017
zfy0701 pushed a commit to lambdalab/php-language-server that referenced this pull request May 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants