Unverified Commit 6532a2be authored by Burak's avatar Burak Committed by GitHub
Browse files

Error out for non-escaped special chars in `@pattern`s (#2752) 

## Motivation and Context
Closes https://github.com/awslabs/smithy-rs/issues/2508

## Testing
Modify `codegen-core/common-test-models/pokemon.smithy`:
```diff
diff --git a/codegen-core/common-test-models/pokemon.smithy b/codegen-core/common-test-models/pokemon.smithy
index 014ee61c4..2dd37046e 100644
--- a/codegen-core/common-test-models/pokemon.smithy
+++ b/codegen-core/common-test-models/pokemon.smithy
@@ -59,9 +59,12 @@ structure GetStorageInput {
     passcode: String,
 }
 
+@pattern("[.\\n\r]+")
+string Species
+
 /// A list of Pokémon species.
 list SpeciesCollection {
-    member: String
+    member: Species
 }
 
 /// Contents of the Pokémon storage.

```

Try to codegen example service and see the error:
```bash
$ cd examples
$ make codegen
...
[ERROR] com.aws.example#Species: Non-escaped special characters used inside `@pattern`.
You must escape them: `@pattern("[.\\n\\r]+")`.
See https://github.com/awslabs/smithy-rs/issues/2508 for more details. | PatternTraitEscapedSpecialChars /smithy-rs/codegen-server-test/../codegen-core/common-test-models/pokemon.smithy:62:1
```
(For some reason validation errors reported by plugins get formatted by
[LineValidationEventFormatter](https://github.com/awslabs/smithy/blob/aca7df7daf31a0e71aebfeb2e72aee06ff707568/smithy-model/src/main/java/software/amazon/smithy/model/validation/LineValidationEventFormatter.java)
but errors reported by smithy-cli formatted by
[PrettyAnsiValidationFormatter](https://github.com/awslabs/smithy/blob/aca7df7daf31a0e71aebfeb2e72aee06ff707568/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/PrettyAnsiValidationFormatter.java).
Might be a bug in smithy-cli)

Replace pattern with `@pattern("[.\\n\\r]+")` and observe that error is
gone:
```bash
$ make codegen
```

## Checklist
<!--- If a checkbox below is not applicable, then please DELETE it
rather than leaving it unchecked -->
- [ ] I have updated `CHANGELOG.next.toml` if I made changes to the
smithy-rs codegen or runtime crates
- [ ] I have updated `CHANGELOG.next.toml` if I made changes to the AWS
SDK, generated SDK code, or SDK runtime crates

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
parent ba726ef1

codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/PatternTraitEscapedSpecialCharsValidator.kt

0 → 100644
+54 −0
Original line number Diff line number Diff line 
/*

 * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.

 * SPDX-License-Identifier: Apache-2.0

 */



package software.amazon.smithy.rust.codegen.server.smithy



import software.amazon.smithy.model.Model

import software.amazon.smithy.model.shapes.Shape

import software.amazon.smithy.model.traits.PatternTrait

import software.amazon.smithy.model.validation.AbstractValidator

import software.amazon.smithy.model.validation.ValidationEvent

import software.amazon.smithy.rust.codegen.core.util.dq

import software.amazon.smithy.rust.codegen.core.util.expectTrait



class PatternTraitEscapedSpecialCharsValidator : AbstractValidator() {

    private val specialCharsWithEscapes = mapOf(

        '\b' to "\\b",

        '\u000C' to "\\f",

        '\n' to "\\n",

        '\r' to "\\r",

        '\t' to "\\t",

    )

    private val specialChars = specialCharsWithEscapes.keys



    override fun validate(model: Model): List<ValidationEvent> {

        val shapes = model.getStringShapesWithTrait(PatternTrait::class.java) +

            model.getMemberShapesWithTrait(PatternTrait::class.java)

        return shapes

            .filter { shape -> checkMisuse(shape) }

            .map { shape -> makeError(shape) }

            .toList()

    }



    private fun makeError(shape: Shape): ValidationEvent {

        val pattern = shape.expectTrait<PatternTrait>()

        val replacement = pattern.pattern.toString()

            .map { specialCharsWithEscapes.getOrElse(it) { it.toString() } }

            .joinToString("")

            .dq()

        val message =

            """

            Non-escaped special characters used inside `@pattern`.

            You must escape them: `@pattern($replacement)`.

            See https://github.com/awslabs/smithy-rs/issues/2508 for more details.

            """.trimIndent()

        return error(shape, pattern, message)

    }



    private fun checkMisuse(shape: Shape): Boolean {

        val pattern = shape.expectTrait<PatternTrait>().pattern.pattern()

        return pattern.any(specialChars::contains)

    }

}

codegen-server/src/main/resources/META-INF/services/software.amazon.smithy.model.validation.Validator

0 → 100644
+5 −0
Original line number Diff line number Diff line 
#

# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.

# SPDX-License-Identifier: Apache-2.0

#

software.amazon.smithy.rust.codegen.server.smithy.PatternTraitEscapedSpecialCharsValidator

codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/PatternTraitEscapedSpecialCharsValidatorTest.kt

0 → 100644
+123 −0
Original line number Diff line number Diff line 
/*

 * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.

 * SPDX-License-Identifier: Apache-2.0

 */



package software.amazon.smithy.rust.codegen.server.smithy



import io.kotest.assertions.throwables.shouldThrow

import io.kotest.matchers.collections.shouldHaveSize

import io.kotest.matchers.shouldBe

import org.junit.jupiter.api.Test

import software.amazon.smithy.model.shapes.ShapeId

import software.amazon.smithy.model.validation.Severity

import software.amazon.smithy.model.validation.ValidatedResultException

import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel



class PatternTraitEscapedSpecialCharsValidatorTest {

    @Test

    fun `should error out with a suggestion if non-escaped special chars used inside @pattern`() {

        val exception = shouldThrow<ValidatedResultException> {

            """

            namespace test

    

            @pattern("\t")

            string MyString

            """.asSmithyModel(smithyVersion = "2")

        }

        val events = exception.validationEvents.filter { it.severity == Severity.ERROR }



        events shouldHaveSize 1

        events[0].shapeId.get() shouldBe ShapeId.from("test#MyString")

        events[0].message shouldBe """

            Non-escaped special characters used inside `@pattern`.

            You must escape them: `@pattern("\\t")`.

            See https://github.com/awslabs/smithy-rs/issues/2508 for more details.

        """.trimIndent()

    }



    @Test

    fun `should suggest escaping spacial characters properly`() {

        val exception = shouldThrow<ValidatedResultException> {

            """

            namespace test

    

            @pattern("[.\n\\r]+")

            string MyString

            """.asSmithyModel(smithyVersion = "2")

        }

        val events = exception.validationEvents.filter { it.severity == Severity.ERROR }



        events shouldHaveSize 1

        events[0].shapeId.get() shouldBe ShapeId.from("test#MyString")

        events[0].message shouldBe """

            Non-escaped special characters used inside `@pattern`.

            You must escape them: `@pattern("[.\\n\\r]+")`.

            See https://github.com/awslabs/smithy-rs/issues/2508 for more details.

        """.trimIndent()

    }



    @Test

    fun `should report all non-escaped special characters`() {

        val exception = shouldThrow<ValidatedResultException> {

            """

            namespace test

    

            @pattern("\b")

            string MyString

            

            @pattern("^\n$")

            string MyString2

            

            @pattern("^[\n]+$")

            string MyString3

            

            @pattern("^[\r\t]$")

            string MyString4

            """.asSmithyModel(smithyVersion = "2")

        }

        val events = exception.validationEvents.filter { it.severity == Severity.ERROR }

        events shouldHaveSize 4

    }



    @Test

    fun `should report errors on string members`() {

        val exception = shouldThrow<ValidatedResultException> {

            """

            namespace test

    

            @pattern("\t")

            string MyString

            

            structure MyStructure {

                @pattern("\b")

                field: String

            }

            """.asSmithyModel(smithyVersion = "2")

        }

        val events = exception.validationEvents.filter { it.severity == Severity.ERROR }



        events shouldHaveSize 2

        events[0].shapeId.get() shouldBe ShapeId.from("test#MyString")

        events[1].shapeId.get() shouldBe ShapeId.from("test#MyStructure\$field")

    }



    @Test

    fun `shouldn't error out if special chars are properly escaped`() {

        """

        namespace test



        @pattern("\\t")

        string MyString

        

        @pattern("[.\\n\\r]+")

        string MyString2

        

        @pattern("\\b\\f\\n\\r\\t")

        string MyString3



        @pattern("\\w+")

        string MyString4

        """.asSmithyModel(smithyVersion = "2")

    }

}