Skip to content

Conversation

@msakai
Copy link
Contributor

@msakai msakai commented Aug 9, 2023

HaskellServantCodegen and HaskellYesodServerCodegen modify specialCharReplacements by replacing its keys: backslash (\) with \\ and " with \".

It seems that those replacements were for using the keys in string literals in the specialChars table in the generated code.
I.e. we want to have ("\\", "Back_Slash") instead of ("\", "Back_Slash") and ("\"", "Double_Quote") instead of (""", "Double_Quote") in the table below.

specialChars =
[ ("$", "Dollar")
, ("^", "Caret")
, ("|", "Pipe")
, ("=", "Equal")
, ("*", "Star")
, ("-", "Dash")
, ("&", "Ampersand")
, ("%", "Percent")
, ("#", "Hash")
, ("@", "At")
, ("!", "Exclamation")
, ("+", "Plus")
, (":", "Colon")
, (";", "Semicolon")
, (">", "GreaterThan")
, ("<", "LessThan")
, (".", "Period")
, ("_", "Underscore")
, ("?", "Question_Mark")
, (",", "Comma")
, ("'", "Quote")
, ("/", "Slash")
, ("(", "Left_Parenthesis")
, (")", "Right_Parenthesis")
, ("{", "Left_Curly_Bracket")
, ("}", "Right_Curly_Bracket")
, ("[", "Left_Square_Bracket")
, ("]", "Right_Square_Bracket")
, ("~", "Tilde")
, ("`", "Backtick")
, ("<=", "Less_Than_Or_Equal_To")
, (">=", "Greater_Than_Or_Equal_To")
, ("!=", "Not_Equal")
, ("<>", "Not_Equal")
, ("~=", "Tilde_Equal")
, ("\\", "Back_Slash")
, ("\"", "Double_Quote")
]

However, modifying the keys causes the substitution of those characters in field names not to work, making generated code syntactically invalid. Suppose we have the following spec:

openapi: 3.0.0
info:
  title: Test
  description: Test API
  version: 1.0.0
servers:
  - url: https://api.example.com/
    description: the server

paths:
  /hello:
    get:
      operationId: hello
      summary: "summary"
      description: "description."
      responses:
        "200":
          description: foo
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/HelloResponse'

components:
  schemas:
    HelloResponse:
      type: object
      properties:
        "\"":
          type: string
          description: double quote
        "\\":
          type: string
          description: backslash
      required:
        - "\""
        - "\\"
      description: description

Those generators generate the following data type definition, but helloResponse" and helloResponse\ are invalid identifiers in Haskell.

-- | description
data HelloResponse = HelloResponse
  { helloResponse" :: Text -- ^ double quote
  , helloResponse\ :: Text -- ^ backslash
  } deriving (Show, Eq, Generic, Data)

Since the specialChars table has already been removed in #16232, we can safely stop modifying the specialCharReplacements, thereby generated code for the above example become the following syntactically correct code.

-- | description
data HelloResponse = HelloResponse
  { helloResponseDoubleQuote :: Text -- ^ double quote
  , helloResponseBackSlash :: Text -- ^ backslash
  } deriving (Show, Eq, Generic, Data)

I believe that this change maintains backward compatibility, as it only fixes cases where it has not been working (when field names contain backslashes or double quotes).

PR checklist

  • Read the contribution guidelines.
  • Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
  • Run the following to build the project and update samples:
    ./mvnw clean package 
    ./bin/generate-samples.sh
    ./bin/utils/export_docs_generators.sh
    
    Commit all changed files.
    This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
    These must match the expectations made by your contribution.
    You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
    For Windows users, please run the script in Git BASH.
  • In case you are adding a new generator, run the following additional script :
    ./bin/utils/ensure-up-to-date
    
    Commit all changed files.
  • File the PR against the correct branch: master (6.3.0) (minor release - breaking changes with fallbacks), 7.0.x (breaking changes without fallbacks)
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

HaskellServantCodegen and HaskellYesodServerCodegen modify
specialCharReplacements by replacing its keys: backslash ("\\") with
"\\\\" and "\"" with "\\\"".

It seems that those replacements were for using the keys in string
literals in the specialChars table in the generated code. However,
modifying the keys causes the substitution of those characters in
field names not to work, making generated code syntactically invalid.

Since the specialChars table has already been removed, we can safely
stop modifying the specialCharReplacements.
@wing328
Copy link
Member

wing328 commented Aug 10, 2023

Tested locally to confirm both the issue and the fix. Thanks for the PR.

➜  haskell-servant stack test
test> configure (lib)
Configuring test-0.1.0.0...
test> build (lib)
Preprocessing library for test-0.1.0.0..
Building library for test-0.1.0.0..
[1 of 2] Compiling Test.Types

/private/tmp/haskell-servant/lib/Test/Types.hs:29:45: error:
    lexical error in string/character literal at character '\n'
   |
29 |   { helloResponse" :: Text -- ^ double quote
   |                                             ^

--  While building package test-0.1.0.0 (scroll up to its section to see the error) using:
      /Users/williamcheng/.stack/setup-exe-cache/x86_64-osx/Cabal-simple_mPHDZzAJ_3.4.1.0_ghc-9.0.2 --builddir=.stack-work/dist/x86_64-osx/Cabal-3.4.1.0 build lib:test --ghc-options " -fdiagnostics-color=always"
    Process exited with code: ExitFailure 1
➜  haskell-servant stack test
test> configure (lib)
Configuring test-0.1.0.0...
test> build (lib)
Preprocessing library for test-0.1.0.0..
Building library for test-0.1.0.0..
[1 of 2] Compiling Test.Types
[2 of 2] Compiling Test.API
test> copy/register
Installing library in /private/tmp/haskell-servant/.stack-work/install/x86_64-osx/d7eaccd255e546d469fd16f5af3892ff7caa92ed8e070d4c3bfe612e97b830c4/9.0.2/lib/x86_64-osx-ghc-9.0.2/test-0.1.0.0-AJ8CDUTcGyD9fnQsiG31e2
Registering library for test-0.1.0.0..
➜  haskell-servant 

I'll file a separate PR to add some tests.

@wing328 wing328 merged commit 9f051ec into OpenAPITools:master Aug 10, 2023
@wing328 wing328 added this to the 7.0.0 milestone Aug 10, 2023
@wing328
Copy link
Member

wing328 commented Aug 10, 2023

Merged #16290 to add some tests and github workflow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants