diff --git a/cli/src/Cli.elm b/cli/src/Cli.elm index 62fdf29..52c6401 100644 --- a/cli/src/Cli.elm +++ b/cli/src/Cli.elm @@ -919,7 +919,7 @@ yamlToJsonValueDecoder = generateFilesFromOpenApiSpecs : - List ( OpenApi.Generate.Config, OpenApi.OpenApi ) + List ( OpenApi.Config.Generate, OpenApi.OpenApi ) -> BackendTask.BackendTask FatalError.FatalError diff --git a/cli/src/TestGenScript.elm b/cli/src/TestGenScript.elm index 3e2bf07..fe3e5fc 100644 --- a/cli/src/TestGenScript.elm +++ b/cli/src/TestGenScript.elm @@ -73,6 +73,7 @@ run = gitHub : OpenApi.Config.Input gitHub = OpenApi.Config.inputFrom (OpenApi.Config.File "./example/github-spec.json") + |> OpenApi.Config.withWarnOnMissingEnums False ifconfigOvh : OpenApi.Config.Input ifconfigOvh = diff --git a/review/suppressed/Docs.NoMissing.json b/review/suppressed/Docs.NoMissing.json index 3afae60..e99318a 100644 --- a/review/suppressed/Docs.NoMissing.json +++ b/review/suppressed/Docs.NoMissing.json @@ -3,7 +3,7 @@ "automatically created by": "elm-review suppress", "learn more": "elm-review suppress --help", "suppressions": [ - { "count": 33, "filePath": "src/OpenApi/Config.elm" }, - { "count": 6, "filePath": "src/OpenApi/Generate.elm" } + { "count": 35, "filePath": "src/OpenApi/Config.elm" }, + { "count": 5, "filePath": "src/OpenApi/Generate.elm" } ] } diff --git a/review/suppressed/NoUnused.Parameters.json b/review/suppressed/NoUnused.Parameters.json deleted file mode 100644 index d1ce0f1..0000000 --- a/review/suppressed/NoUnused.Parameters.json +++ /dev/null @@ -1,8 +0,0 @@ -{ - "version": 1, - "automatically created by": "elm-review suppress", - "learn more": "elm-review suppress --help", - "suppressions": [ - { "count": 1, "filePath": "src/SchemaUtils.elm" } - ] -} diff --git a/src/CliMonad.elm b/src/CliMonad.elm index edfca08..bda1da6 100644 --- a/src/CliMonad.elm +++ b/src/CliMonad.elm @@ -76,6 +76,7 @@ type CliMonad a , enums : FastDict.Dict (List String) { name : Common.UnsafeName, documentation : Maybe String } , namespace : List String , formats : FastDict.Dict InternalFormatName InternalFormat + , warnOnMissingEnums : Bool } -> Result Message ( a, Output ) ) @@ -284,6 +285,7 @@ run : , enums : FastDict.Dict (List String) { name : Common.UnsafeName, documentation : Maybe String } , namespace : List String , formats : List OpenApi.Config.Format + , warnOnMissingEnums : Bool } -> CliMonad (List Declaration) -> @@ -301,6 +303,7 @@ run oneOfDeclarations input (CliMonad x) = , enums : FastDict.Dict (List String) { name : Common.UnsafeName, documentation : Maybe String } , namespace : List String , formats : FastDict.Dict InternalFormatName InternalFormat + , warnOnMissingEnums : Bool } internalInput = { openApi = input.openApi @@ -311,6 +314,7 @@ run oneOfDeclarations input (CliMonad x) = input.formats |> List.map toInternalFormat |> FastDict.fromList + , warnOnMissingEnums = input.warnOnMissingEnums } in x internalInput @@ -441,20 +445,32 @@ enumName variants = succeed (Just name) Nothing -> - succeed Nothing - |> withWarning - ("No named enum found for [ " - ++ String.join ", " - (List.map - (\variant -> - variant - |> Common.unwrapUnsafe - |> escapeString - ) - variants - ) - ++ " ]. Define one to improve type safety" - ) + CliMonad + (\{ warnOnMissingEnums } -> + if warnOnMissingEnums then + let + variantNames : List String + variantNames = + List.map + (\variant -> + variant + |> Common.unwrapUnsafe + |> escapeString + ) + variants + + message : String + message = + "No named enum found for [ " + ++ String.join ", " variantNames + ++ " ]. Define one to improve type safety" + in + ( Nothing, { emptyOutput | warnings = [ { path = [], message = message } ] } ) + |> Ok + + else + Ok ( Nothing, emptyOutput ) + ) ) enums @@ -548,7 +564,11 @@ withFormat basicType maybeFormatName getter default = secondLine : String secondLine = - " Available formats: " ++ String.join ", " available + if List.isEmpty available then + " No available formats." + + else + " Available formats: " ++ String.join ", " available ++ "." in { emptyOutput | warnings = diff --git a/src/OpenApi/Config.elm b/src/OpenApi/Config.elm index 6d76876..af716f8 100644 --- a/src/OpenApi/Config.elm +++ b/src/OpenApi/Config.elm @@ -2,10 +2,10 @@ module OpenApi.Config exposing ( Config, EffectType(..), effectTypeToPackage, Format, Input, Path(..), Server(..) , init, inputFrom, pathFromString , withAutoConvertSwagger, withEffectTypes, withFormat, withFormats, withGenerateTodos, withInput, withSwaggerConversionCommand, withSwaggerConversionUrl - , withOutputModuleName, withOverrides, withServer, withWriteMergedTo + , withOutputModuleName, withOverrides, withServer, withWriteMergedTo, withWarnOnMissingEnums , autoConvertSwagger, inputs, outputDirectory, swaggerConversionCommand, swaggerConversionUrl , oasPath, overrides, writeMergedTo - , toGenerationConfig, pathToString + , toGenerationConfig, Generate, pathToString , defaultFormats ) @@ -21,7 +21,7 @@ module OpenApi.Config exposing @docs init, inputFrom, pathFromString @docs withAutoConvertSwagger, withEffectTypes, withFormat, withFormats, withGenerateTodos, withInput, withSwaggerConversionCommand, withSwaggerConversionUrl -@docs withOutputModuleName, withOverrides, withServer, withWriteMergedTo +@docs withOutputModuleName, withOverrides, withServer, withWriteMergedTo, withWarnOnMissingEnums # Config properties @@ -36,7 +36,7 @@ module OpenApi.Config exposing # Output -@docs toGenerationConfig, pathToString +@docs toGenerationConfig, Generate, pathToString # Internal @@ -92,6 +92,7 @@ type Input , overrides : List Path , writeMergedTo : Maybe String , effectTypes : List EffectType + , warnOnMissingEnums : Bool } @@ -232,6 +233,7 @@ inputFrom path = , overrides = [] , writeMergedTo = Nothing , effectTypes = [ ElmHttpCmd, ElmHttpTask ] + , warnOnMissingEnums = False } |> Input @@ -472,6 +474,12 @@ withWriteMergedTo newWriteMergedTo (Input input) = Input { input | writeMergedTo = Just newWriteMergedTo } +{-| -} +withWarnOnMissingEnums : Bool -> Input -> Input +withWarnOnMissingEnums newWarnOnMissingEnums (Input input) = + Input { input | warnOnMissingEnums = newWarnOnMissingEnums } + + {-| -} withFormat : Format -> Config -> Config withFormat newFormat (Config config) = @@ -553,6 +561,17 @@ overrides (Input input) = ------------ +{-| -} +type alias Generate = + { namespace : List String + , generateTodos : Bool + , effectTypes : List EffectType + , server : Server + , formats : List Format + , warnOnMissingEnums : Bool + } + + {-| -} toGenerationConfig : List { format : String, basicType : Common.BasicType } @@ -560,12 +579,7 @@ toGenerationConfig : -> List ( Input, OpenApi.OpenApi ) -> List - ( { namespace : List String - , generateTodos : Bool - , effectTypes : List EffectType - , server : Server - , formats : List Format - } + ( Generate , OpenApi.OpenApi ) toGenerationConfig formatsInput (Config config) augmentedInputs = @@ -586,6 +600,7 @@ toGenerationConfig formatsInput (Config config) augmentedInputs = , generateTodos = config.generateTodos , effectTypes = input.effectTypes , server = input.server + , warnOnMissingEnums = input.warnOnMissingEnums , formats = config.staticFormats ++ config.dynamicFormats formatsInput } , spec diff --git a/src/OpenApi/Generate.elm b/src/OpenApi/Generate.elm index 46ed659..71234df 100644 --- a/src/OpenApi/Generate.elm +++ b/src/OpenApi/Generate.elm @@ -1,8 +1,8 @@ -module OpenApi.Generate exposing (Config, ContentSchema(..), Message, Path, Mime, files) +module OpenApi.Generate exposing (ContentSchema(..), Message, Path, Mime, files) {-| -@docs Config, ContentSchema, Message, Path, Mime, files +@docs ContentSchema, Message, Path, Mime, files -} @@ -100,19 +100,9 @@ type alias PerPackage a = } -{-| -} -type alias Config = - { namespace : List String - , generateTodos : Bool - , effectTypes : List OpenApi.Config.EffectType - , server : OpenApi.Config.Server - , formats : List OpenApi.Config.Format - } - - {-| -} files : - Config + OpenApi.Config.Generate -> OpenApi.OpenApi -> Result @@ -125,7 +115,7 @@ files : , warnings : List Message , requiredPackages : FastSet.Set String } -files { namespace, generateTodos, effectTypes, server, formats } apiSpec = +files { namespace, generateTodos, effectTypes, server, formats, warnOnMissingEnums } apiSpec = case extractEnums apiSpec of Err e -> Err e @@ -151,6 +141,7 @@ files { namespace, generateTodos, effectTypes, server, formats } apiSpec = , enums = enums , namespace = namespace , formats = formats + , warnOnMissingEnums = warnOnMissingEnums } |> Result.map (\{ declarations, warnings, requiredPackages } -> diff --git a/src/SchemaUtils.elm b/src/SchemaUtils.elm index 9fbf1af..8e3e576 100644 --- a/src/SchemaUtils.elm +++ b/src/SchemaUtils.elm @@ -245,8 +245,20 @@ schemaToType qualify schema = (schemaToType qualify itemSchema) anyOfToType : List Json.Schema.Definitions.Schema -> CliMonad { type_ : Common.Type, documentation : Maybe String } - anyOfToType _ = - CliMonad.succeed { type_ = Common.Value, documentation = subSchema.description } + anyOfToType schemas = + areSchemasDisjoint qualify schemas + |> CliMonad.andThen + (\disjoint -> + if disjoint then + oneOfToType schemas + + else + CliMonad.succeed + { type_ = Common.Value + , documentation = subSchema.description + } + |> CliMonad.withWarning "anyOf between overlapping types is not supported" + ) oneOfCombine : List Json.Schema.Definitions.Schema -> CliMonad { type_ : Common.Type, documentation : Maybe String } oneOfCombine oneOf = @@ -428,6 +440,407 @@ schemaToType qualify schema = ) +areSchemasDisjoint : Bool -> List Json.Schema.Definitions.Schema -> CliMonad Bool +areSchemasDisjoint qualify schemas = + let + areDisjoint : Json.Schema.Definitions.Schema -> Json.Schema.Definitions.Schema -> CliMonad Bool + areDisjoint l r = + case ( l, r ) of + ( Json.Schema.Definitions.BooleanSchema lb, Json.Schema.Definitions.BooleanSchema rb ) -> + if lb == rb then + CliMonad.succeed False + |> CliMonad.withWarning "anyOf between two booleans with the same value" + + else + CliMonad.succeed True + + ( Json.Schema.Definitions.BooleanSchema _, Json.Schema.Definitions.ObjectSchema _ ) -> + CliMonad.succeed True + + ( Json.Schema.Definitions.ObjectSchema _, Json.Schema.Definitions.BooleanSchema _ ) -> + CliMonad.succeed True + + ( Json.Schema.Definitions.ObjectSchema lo, Json.Schema.Definitions.ObjectSchema ro ) -> + CliMonad.andThen2 + (\lprop rprop -> + let + ldict : FastDict.Dict String Common.Field + ldict = + lprop + |> List.map + (\( name, field ) -> + ( Common.unwrapUnsafe name, field ) + ) + |> FastDict.fromList + + ladd : Bool + ladd = + case lo.additionalProperties of + Just (Json.Schema.Definitions.BooleanSchema False) -> + False + + _ -> + True + + rdict : FastDict.Dict String Common.Field + rdict = + rprop + |> List.map + (\( name, field ) -> + ( Common.unwrapUnsafe name, field ) + ) + |> FastDict.fromList + + radd : Bool + radd = + case ro.additionalProperties of + Just (Json.Schema.Definitions.BooleanSchema False) -> + False + + _ -> + True + + merged : Result (List String) () + merged = + FastDict.merge + (\_ lval prev -> + case prev of + Ok () -> + prev + + Err _ -> + -- A required field on the left when additionalProperties are forbidden on the right means the sets are disjoint + if lval.required && not radd then + Ok () + + else + prev + ) + (\_ lval rval prev -> + case prev of + Ok () -> + prev + + Err warns -> + -- If the field is optional in both we could have a value without it, so it's not enough to distinguish, so we ask it's required in at least one of them + if lval.required || rval.required then + let + ( res, newWarns ) = + areTypesDisjoint lval.type_ rval.type_ + in + if res then + Ok () + + else + Err (warns ++ newWarns) + + else + prev + ) + (\_ rval prev -> + case prev of + Ok () -> + prev + + Err _ -> + -- A required field on the right when additionalProperties are forbidden on the left means the sets are disjoint + if rval.required && not ladd then + Ok () + + else + prev + ) + ldict + rdict + (Err []) + in + case merged of + Ok () -> + CliMonad.succeed True + + Err warns -> + warns + |> Set.fromList + |> Set.foldl + CliMonad.withWarning + (CliMonad.succeed False) + ) + (schemaToProperties qualify l) + (schemaToProperties qualify r) + + go : + Json.Schema.Definitions.Schema + -> List Json.Schema.Definitions.Schema + -> CliMonad Bool + go item queue = + queue + |> CliMonad.combineMap (\other -> areDisjoint item other) + |> CliMonad.andThen + (\r -> + if List.all identity r then + case queue of + [] -> + CliMonad.succeed True + + h :: t -> + go h t + + else + CliMonad.succeed False + ) + in + case schemas of + [] -> + CliMonad.succeed True + + h :: t -> + go h t + + +type SimplifiedForDisjointBasicType + = SimplifiedForDisjointNumber (Maybe Float) + | SimplifiedForDisjointString (Maybe String) + | SimplifiedForDisjointBool (Maybe Bool) + + +areTypesDisjoint : Common.Type -> Common.Type -> ( Bool, List String ) +areTypesDisjoint ltype rtype = + case ( ltype, rtype ) of + ( Common.Ref _, _ ) -> + ( False, [ "Disjoin check for ref types not implemented yet" ] ) + + ( _, Common.Ref _ ) -> + ( False, [ "Disjoin check for ref types not implemented yet" ] ) + + ( Common.Value, _ ) -> + ( False, [] ) + + ( _, Common.Value ) -> + ( False, [] ) + + ( Common.Nullable _, Common.Nullable _ ) -> + ( False, [] ) + + ( Common.Null, Common.Nullable _ ) -> + ( False, [] ) + + ( Common.Nullable _, Common.Null ) -> + ( False, [] ) + + ( Common.Nullable c, Common.Basic _ _ ) -> + areTypesDisjoint c rtype + + ( Common.Basic _ _, Common.Nullable c ) -> + areTypesDisjoint ltype c + + ( Common.Null, Common.Null ) -> + ( False, [] ) + + ( Common.Null, Common.List _ ) -> + ( True, [] ) + + ( Common.List _, Common.Null ) -> + ( True, [] ) + + ( Common.OneOf _ alternatives, _ ) -> + alternatives + |> List.map (\alternative -> areTypesDisjoint alternative.type_ rtype) + |> List.unzip + |> Tuple.mapBoth (List.all identity) List.concat + + ( _, Common.OneOf _ alternatives ) -> + alternatives + |> List.map (\alternative -> areTypesDisjoint ltype alternative.type_) + |> List.unzip + |> Tuple.mapBoth (List.all identity) List.concat + + ( Common.List _, Common.List _ ) -> + -- Empty lists are not distinguished + ( False, [] ) + + ( Common.Basic lbasic lopt, Common.Basic rbasic ropt ) -> + case + ( simplifyForDisjoint lbasic lopt.const + , simplifyForDisjoint rbasic ropt.const + ) + of + ( Err warning, _ ) -> + ( False, [ warning ] ) + + ( _, Err warning ) -> + ( False, [ warning ] ) + + ( Ok (SimplifiedForDisjointBool lconst), Ok (SimplifiedForDisjointBool rconst) ) -> + ( lconst /= rconst, [] ) + + ( Ok (SimplifiedForDisjointNumber lconst), Ok (SimplifiedForDisjointNumber rconst) ) -> + ( lconst /= rconst, [] ) + + ( Ok (SimplifiedForDisjointString lconst), Ok (SimplifiedForDisjointString rconst) ) -> + if lconst /= rconst then + ( True, [] ) + + else + -- TODO: check for disjoint formats + ( False, [] ) + + _ -> + ( True, [] ) + + ( Common.Object lfields, Common.Object rfields ) -> + let + ldict : FastDict.Dict String Common.Field + ldict = + lfields + |> List.map (\( k, v ) -> ( Common.unwrapUnsafe k, v )) + |> FastDict.fromList + + rdict : FastDict.Dict String Common.Field + rdict = + rfields + |> List.map (\( k, v ) -> ( Common.unwrapUnsafe k, v )) + |> FastDict.fromList + in + FastDict.merge + (\_ _ acc -> acc) + (\_ lfield rfield ( acc, warns ) -> + if acc || (not lfield.required && not rfield.required) then + ( acc, warns ) + + else + let + ( nacc, nwarns ) = + areTypesDisjoint lfield.type_ rfield.type_ + in + ( nacc, warns ++ nwarns ) + ) + (\_ _ acc -> acc) + ldict + rdict + ( False, [] ) + + ( Common.Enum lItems, Common.Enum rItems ) -> + let + lSet : Set String + lSet = + lItems + |> List.map Common.unwrapUnsafe + |> Set.fromList + + rSet : Set String + rSet = + rItems + |> List.map Common.unwrapUnsafe + |> Set.fromList + in + ( Set.isEmpty (Set.intersect lSet rSet), [] ) + + ( Common.Enum lItems, Common.Basic Common.String rOptions ) -> + case rOptions.const of + Just (Common.ConstString rConst) -> + ( List.all (\lItem -> Common.unwrapUnsafe lItem /= rConst) lItems, [] ) + + Just _ -> + ( False, [ "Wrong constant type" ] ) + + Nothing -> + case rOptions.format of + Nothing -> + ( False, [] ) + + Just rFormat -> + ( False, [ "Disjoin check not implemented for types enum and string:" ++ rFormat ] ) + + ( Common.Basic Common.String _, Common.Enum _ ) -> + areTypesDisjoint rtype ltype + + _ -> + ( False, [ "Disjoin check not implemented for types " ++ typeToString ltype ++ " and " ++ typeToString rtype ] ) + + +typeToString : Common.Type -> String +typeToString type_ = + case type_ of + Common.Null -> + "null" + + Common.Nullable _ -> + "nullable" + + Common.Object _ -> + "object" + + Common.Basic _ _ -> + "basic" + + Common.List _ -> + "list" + + Common.Dict _ _ -> + "dict" + + Common.OneOf _ _ -> + "oneOf" + + Common.Enum _ -> + "enum" + + Common.Value -> + "value" + + Common.Ref _ -> + "ref" + + Common.Bytes -> + "bytes" + + Common.Unit -> + "unit" + + +simplifyForDisjoint : Common.BasicType -> Maybe Common.ConstValue -> Result String SimplifiedForDisjointBasicType +simplifyForDisjoint basic const = + case ( basic, const ) of + ( Common.Boolean, Nothing ) -> + Ok (SimplifiedForDisjointBool Nothing) + + ( Common.Boolean, Just (Common.ConstBoolean b) ) -> + Ok (SimplifiedForDisjointBool (Just b)) + + ( Common.Boolean, Just _ ) -> + Err "Invalid const for boolean type" + + ( Common.String, Nothing ) -> + Ok (SimplifiedForDisjointString Nothing) + + ( Common.String, Just (Common.ConstString b) ) -> + Ok (SimplifiedForDisjointString (Just b)) + + ( Common.String, Just _ ) -> + Err "Invalid const for string type" + + ( Common.Integer, Nothing ) -> + Ok (SimplifiedForDisjointNumber Nothing) + + ( Common.Integer, Just (Common.ConstInteger i) ) -> + Ok (SimplifiedForDisjointNumber (Just (toFloat i))) + + ( Common.Integer, Just _ ) -> + Err "Invalid const for integer type" + + ( Common.Number, Nothing ) -> + Ok (SimplifiedForDisjointNumber Nothing) + + ( Common.Number, Just (Common.ConstInteger i) ) -> + Ok (SimplifiedForDisjointNumber (Just (toFloat i))) + + ( Common.Number, Just (Common.ConstNumber f) ) -> + Ok (SimplifiedForDisjointNumber (Just f)) + + ( Common.Number, Just _ ) -> + Err "Invalid const for number type" + + subschemaToEnumMaybe : Json.Schema.Definitions.SubSchema -> @@ -622,29 +1035,40 @@ objectSchemaToType qualify subSchema = declaredProperties additionalSchema in - case subSchema.additionalProperties of - -- Object contains only specified properties, not arbitrary extra ones. - Just (Json.Schema.Definitions.BooleanSchema False) -> - declaredProperties - - -- It's unclear whether "true" is *technically* an acceptable value for - -- additionalProperties, but the GitHub API spec uses it. Since the type - -- of the properties could be anything, we keep them as Json Values. - Just (Json.Schema.Definitions.BooleanSchema True) -> - CliMonad.succeed - { type_ = Common.Value - , documentation = Just "Arbitrary data whose type is not defined by the API spec" - } - |> declaredAndAdditionalProperties - - -- The object contains an additionalProperties entry that describes the - -- type of the values, which may have arbitrary keys, in the object. - Just additionalPropertiesSchema -> - schemaToType qualify additionalPropertiesSchema - |> declaredAndAdditionalProperties + (case subSchema.patternProperties of + Just _ -> + CliMonad.succeed () + |> CliMonad.withWarning "patternProperties not implemented yet" Nothing -> - declaredProperties + CliMonad.succeed () + ) + |> CliMonad.andThen + (\() -> + case subSchema.additionalProperties of + -- Object contains only specified properties, not arbitrary extra ones. + Just (Json.Schema.Definitions.BooleanSchema False) -> + declaredProperties + + -- It's unclear whether "true" is *technically* an acceptable value for + -- additionalProperties, but the GitHub API spec uses it. Since the type + -- of the properties could be anything, we keep them as Json Values. + Just (Json.Schema.Definitions.BooleanSchema True) -> + CliMonad.succeed + { type_ = Common.Value + , documentation = Just "Arbitrary data whose type is not defined by the API spec" + } + |> declaredAndAdditionalProperties + + -- The object contains an additionalProperties entry that describes the + -- type of the values, which may have arbitrary keys, in the object. + Just additionalPropertiesSchema -> + schemaToType qualify additionalPropertiesSchema + |> declaredAndAdditionalProperties + + Nothing -> + declaredProperties + ) joinIfNotEmpty : String -> List (Maybe String) -> Maybe String diff --git a/tests/Test/OpenApi/Generate.elm b/tests/Test/OpenApi/Generate.elm index a7f882b..d2ad49c 100644 --- a/tests/Test/OpenApi/Generate.elm +++ b/tests/Test/OpenApi/Generate.elm @@ -117,6 +117,7 @@ suite = , effectTypes = [ OpenApi.Config.ElmHttpCmd, OpenApi.Config.ElmHttpTask ] , server = OpenApi.Config.Default , formats = OpenApi.Config.defaultFormats + , warnOnMissingEnums = True } oas in