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

Collision detection #412

Merged
merged 49 commits into from
Sep 1, 2022
Merged

Collision detection #412

merged 49 commits into from
Sep 1, 2022

Conversation

yisraelU
Copy link
Contributor

Draft to address collisions with stdlib types and keywords
keyword protection is done with underscore as using backticks (which would be idiomatic scala) creates a new problem where we append to names "Gen" or "Op" etc.. a backtick must wrap the entire identifier
I found this to be sufficient reason to avoid switching to backticks as it would involve significant code shuffling .
This draft is to be reviewed for correctness . I will be making further commits to address code conciseness and quality

…l information until collision detection and handling is done
# Conflicts:
#	modules/codegen/src/smithy4s/codegen/Renderer.scala
#	modules/example/src/smithy4s/example/GetObjectInput.scala
#	modules/example/src/smithy4s/example/LowHigh.scala
#	modules/example/src/smithy4s/example/ObjectService.scala
#	modules/example/src/smithy4s/example/PutObjectInput.scala
#	modules/example/src/smithy4s/example/StreamedObjects.scala
#	modules/example/src/smithy4s/example/StructureWithRefinedTypes.scala
#	modules/example/src/smithy4s/example/import_test/OpOutput.scala
2. renames ADT for modeling the code and imports
3. renderGetMessage returns Line
4. regenerate examples
added predef and package object scala imports to collision detection
added test case to handle Set,List,Option,Map
@Baccata
Copy link
Contributor

Baccata commented Aug 25, 2022

Ain't got time to review proper before next week, but, I think the most desirable solution would be to remove the underscore-logic altogether in favour of a local rename.

Say you have the following in a smithy model

namespace foo 

@uniqueItems 
list Set {
    member: String
} 

then you'll have foo.Set colliding with scala.Set. When handling such things manually, what we'd do is :

import scala.{Set => SSet} 

Doing this would be facilitated by the fact that we're retaining structural information for longer, meaning we can transform the NameRefs everywhere they need to be before collapsing the structure into text.

@yisraelU
Copy link
Contributor Author

@Baccata I'm not using underscore for collision renaming.
It's only being used for keyword renaming i.e. a val name type

@yisraelU
Copy link
Contributor Author

this will fix #255 too

yisraelU and others added 11 commits August 29, 2022 15:17
Adds logic to protect against rendering types using (non-fqn) names that
Scala devs are bound to use when using smithy4s. (stdlib, smithy.Timestamp, etc).

For instance, when encountering `foo#List`, then a `_List` scala type gets created
instead.

If `List` is the name of an operation, then the method created is `list`, in an
effort to minimise the impact on user experience.
@Baccata Baccata marked this pull request as ready for review August 30, 2022 17:02
 Conflicts:
	modules/example/src/smithy4s/example/package.scala
@daddykotex daddykotex merged commit c7a86d6 into main Sep 1, 2022
@Baccata Baccata deleted the collision-detection branch September 1, 2022 07:24
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.

4 participants