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 rebuilding when SourceLocation is NONE #196

Merged

Conversation

milesziemer
Copy link
Contributor

The rebuild index checks which shapes have traits that are applied in other files (i.e. via an apply), so we can preserve those traits in a reload. However, if a trait's source location is SourceLocation.NONE, which can happen if it wasn't set on the trait's builder, the rebuild index would consider that "a trait applied from another file". If the trait isn't actually being applied from another file, this would cause a duplicate trait conflict, since the trait would be added both from the rebuild index, and the ModelAssembler reparsing.

This commit changes two things:

  1. The trait's node source location is now used for the comparison, since it should always have a source location as it is constructed within smithy-model, not by trait provider implementations
  2. This source location is checked to see if it is NONE.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@milesziemer milesziemer requested a review from a team as a code owner February 13, 2025 20:23
@milesziemer milesziemer requested a review from yefrig February 13, 2025 20:23
The rebuild index checks which shapes have traits that are applied in
other files (i.e. via an `apply`), so we can preserve those traits in a
reload. However, if a trait's source location is `SourceLocation.NONE`,
which can happen if it wasn't set on the trait's builder, the rebuild
index would consider that "a trait applied from another file". If the
trait isn't actually being applied from another file, this would cause
a duplicate trait conflict, since the trait would be added both from the
rebuild index, and the ModelAssembler reparsing.

This commit changes two things:
1. The trait's _node_ source location is now used for the comparison,
   since it should always have a source location as it is constructed
   within smithy-model, not by trait provider implementations
2. This source location is checked to see if it is NONE.
@milesziemer milesziemer force-pushed the fix-rebuild-for-sourcelocation-none branch from ef20936 to da56c27 Compare February 13, 2025 21:23
@milesziemer milesziemer merged commit 7ddd163 into smithy-lang:main Feb 13, 2025
3 checks passed
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.

2 participants