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

ModelTransformer.mapShapes ignores location changes #2508

Open
kubukoz opened this issue Feb 6, 2025 · 1 comment
Open

ModelTransformer.mapShapes ignores location changes #2508

kubukoz opened this issue Feb 6, 2025 · 1 comment

Comments

@kubukoz
Copy link
Contributor

kubukoz commented Feb 6, 2025

When you use mapShapes to update your shapes' sourceLocation, it appears to be ignored.

Demo (scala-cli snippet):

//> using scala 3.6.3
//> using dep software.amazon.smithy:smithy-model:1.54.0
import software.amazon.smithy.model.Model
import software.amazon.smithy.model.transform.ModelTransformer
import software.amazon.smithy.model.shapes.Shape
import software.amazon.smithy.model.shapes.AbstractShapeBuilder
import software.amazon.smithy.model.shapes.StructureShape

def transform(s: Shape): Shape = {
  val sb: AbstractShapeBuilder[?, ?] = Shape
    .shapeToBuilder(s)

  sb
    .source("foo", 1, 2)
    .build()
}

val struct = StructureShape
  .builder()
  .id("foo#bar")
  .build()
// struct: StructureShape = (structure: `foo#bar`)

transform(struct).getSourceLocation()
// res0: SourceLocation = foo [1, 2]

val baseModel = Model.builder().addShape(struct).build()
// baseModel: Model = software.amazon.smithy.model.Model@5eab45b

// doesn't get updated with mapShapes
ModelTransformer
  .create()
  .mapShapes(
    baseModel,
    transform(_),
  )
  .expectShape(struct.getId())
  .getSourceLocation()
// res1: SourceLocation = N/A [0, 0]

// but it works when you create a model from scratch
Model
  .builder()
  .addShapes(
    baseModel.shapes().map(transform).toList()
  )
  .build()
  .expectShape(struct.getId())
  .getSourceLocation()
// res2: SourceLocation = foo [1, 2]
@mtdowling
Copy link
Member

I don't have a solution, but I'm pretty sure this is caused by the fact that shape equality is not impacted by SourceLocation. So when mapShapes checks the return value, it sees the shape hasn't changed, and doesn't update it. This is the right behavior shape equality, but we might need something to account for this in the transform specifically.

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

No branches or pull requests

2 participants