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.

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

Fix warning for nested arrays with custom types #11

Merged
merged 4 commits into from Nov 21, 2020

Conversation

KaanOzkan
Copy link
Contributor

@KaanOzkan KaanOzkan commented Nov 20, 2020

I've been encountering warnings such as:
@param tag has unknown parameter name: ] or @param tag has unknown parameter name: ,

I noticed that this only occurs with signatures that contain nested custom types such as T::Array[Custom[String]].

After debugging I noticed that the inner node Custom[String] is actually parsed as Custom[String]] within Yard parser.

So while converting T::Array[Custom[String]] node we correctly convert the outer T::Array and call convert with its children, the inner Custom[String]] node. Then we enter the else branch of the :aref case and return Custom[String]] which results in the warning.

I noticed that the parser returns an extra ] even for T::Array[String] signature. This case is not outputting a warning because we extract the relevant information here.

I wasn't 100% sure how to fix this so I'm open to suggestions. Currently I opted for a similar approach as the T::Array case. As you can see from the test cases for an input of T::Array[Custom[String]] we would output Array<Custom[String]> .

This could be wrong if we encountered "non array" signatures inside that else block. I'm curious if that is a possibility given we are in a :aref node

@codecov
Copy link

codecov bot commented Nov 20, 2020

Codecov Report

Merging #11 (910c233) into master (7a1cb1d) will increase coverage by 0.38%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #11      +/-   ##
==========================================
+ Coverage   92.44%   92.82%   +0.38%     
==========================================
  Files           2        2              
  Lines         225      237      +12     
==========================================
+ Hits          208      220      +12     
  Misses         17       17              
Impacted Files Coverage Δ
spec/yard_sorbet/sig_handler_spec.rb 99.05% <100.00%> (+0.05%) ⬆️

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 7a1cb1d...910c233. Read the comment docs.

@dduugg
Copy link
Owner

dduugg commented Nov 21, 2020

Nice debugging story! Might be worth filing an issue in yard itself (but we're possibly using it in an supported way).

@dduugg dduugg merged commit 46b2d05 into dduugg:master Nov 21, 2020
@KaanOzkan KaanOzkan deleted the ko/fix-custom-nested-array branch November 23, 2020 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants