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

Stop detecting longform Optional expression types. #135

Merged
merged 1 commit into from
Jan 1, 2025

Conversation

dfed
Copy link
Owner

@dfed dfed commented Jan 1, 2025

In #77, I added code to find Optional<Generic>.self and treat the reference as Generic?.self. This detection was added so that we could throw an error when fulfillingAdditionalTypes was written as a longform optional (Optional<Generic>) rather than a shortform optional (Generic?).

However, this detection can be wrong. If someone were to define a nested Optional type and then refer to it in fulfillingAdditionalTypes without qualification, we'd parse it incorrectly. Fixing this issue would require having locally defined type context at the var typeDescription: TypeDescription scope, which isn't feasible.

While this particular issue is likely quite rare, it's not worth introducing this kind of trap door in order to protect consumers from trying to fulfillingAdditionalTypes with a longform optional: there are all kinds of potential fulfillingAdditionalTypes failures—like fulfilling non-subclassable concrete types—that we do not error on because we do not have enough information to detect it.

I also updated placement of return while I was here.

Also updated placement of return
@dfed dfed self-assigned this Jan 1, 2025
Copy link

codecov bot commented Jan 1, 2025

Codecov Report

Attention: Patch coverage is 92.30769% with 2 lines in your changes missing coverage. Please review.

Project coverage is 99.91%. Comparing base (1d19827) to head (37be0f5).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
Sources/SafeDICore/Models/TypeDescription.swift 92.30% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #135      +/-   ##
==========================================
- Coverage   99.91%   99.91%   -0.01%     
==========================================
  Files          32       32              
  Lines        3475     3467       -8     
==========================================
- Hits         3472     3464       -8     
  Misses          3        3              
Files with missing lines Coverage Δ
Sources/SafeDICore/Models/TypeDescription.swift 99.56% <92.30%> (-0.01%) ⬇️

@dfed dfed marked this pull request as ready for review January 1, 2025 20:41
@dfed dfed merged commit 1ca775d into main Jan 1, 2025
13 checks passed
@dfed dfed deleted the dfed--longform-optional-removal branch January 1, 2025 20:41
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.

1 participant