Printers review (#66)

This PR fixes decoders/encoders for `allOf` and `anyOf` types, sanitises identifier names, various bug fixes, refactorings and dialyzer fixes. * Fixes and refactors the `allOf` and `anyOf` printers, such that they produce correct Elm code for decoding and encoding `all_of` and `any_of` JSON schema nodes. * Sanitises / Elmifies JSON schema identifiers, such that the Elm code output is valid Elm - Moves indentation- and naming-specific logic from `printer/util.ex` into a new `printer/utils` folder, - adjusts all printers to now use the `Naming.normalize_identifier()` function before sending identifier names to Elm code templates, and - adds tests for the 'sanitise identifier' feature. * Splits the `JS2E.Printer.Util` module into a whole `printer/utils` folder in order to increase cohesion, i.e. have one util module per relevant area of printing. * Fixes all dialyzer errors except for allOf/anyOf/oneOf printers * Improves documentation - Updates 'allOf' and 'anyOf' type descriptions to reflect new Elm decoders/encoders, - updates `README.md` to include a section on `js2e` error reporting, and - creates a `CONTRIBUTING.md` file, detailing what potential contributors should know before filing issues/PRs.
dragonwasrobot · Apr 29, 2018 · 0cd9f0d · 0cd9f0d
1 parent b14b302
commit 0cd9f0d
Show file tree

Hide file tree

Showing 54 changed files with 2,141 additions and 1,440 deletions.
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -0,0 +1,67 @@
+# Contributing
+
+If you have found a bug, think I have misinterpreted the JSON schema spec
+somewhere, or have a proposal for a new feature, feel free to open an issue so
+we can discuss a proper solution.
+
+## Reporting bugs
+
+When reporting a bug, please include:
+
+- A short description of the bug,
+- JSON schema example that triggers the bug,
+- expected Elm output, and the
+- actual Elm output.
+
+## Pull requests
+
+Please do not create pull requests before an issue has been created and a
+solution has been discussed and agreed upon.
+
+When making a pull request ensure that:
+
+1. It solves one specific problem, and that problem has already been documented
+   and discussed as an issue,
+2. the PR solves the problem as agreed upon in the issue,
+3. if it is a new feature, ensure that there is proper code coverage of the new
+   feature, and
+4. the PR contains no compiler warnings or dialyzer warnings (and preferably no
+   credo warnings).
+
+## Development
+
+The project is written in [Elixir](http://elixir-lang.org/) - as I found it to
+be a more suitable tool for the job than Elm - and uses the `mix` tool for
+building.
+
+#### Compiling
+
+Install dependencies
+
+    mix deps.get
+
+Compile project
+
+    mix compile
+
+and you are good to go.
+
+#### Tests
+
+Run the standard mix task
+
+    mix test
+
+for test coverage run
+
+    mix coveralls.html
+
+#### Static analysis
+
+Run dialyzer
+
+    mix dialyzer
+
+Run credo
+
+    mix credo
diff --git a/README.md b/README.md
@@ -277,35 +277,51 @@ encodeCircle circle =
                 ++ radius
 ```
 
-## Contributing
+## Error reporting
 
-If you feel like something is missing/wrong or if I've misinterpreted the JSON
-schema spec, feel free to open an issue so we can discuss a solution.
+Any errors encountered by the `js2e` tool while parsing the JSON schema files or
+printing the Elm code output, is reported in an Elm-like style, e.g.
+
+```
+--- UNKNOWN NODE TYPE -------------------------------------- all_of_example.json
+
+The value of "type" at '#/allOf/0/properties/description' did not match a known node type
+
+    "type": "strink"
+            ^^^^^^^^
 
-### Development
+Was expecting one of the following types
 
-As noted in the installation section, the project is written
-in [Elixir](http://elixir-lang.org/) - as I found it to be a more suitable tool
-for the job than Elm, and uses the `mix` tool for building.
+    ["null", "boolean", "object", "array", "number", "integer", "string"]
 
-#### Compiling
+Hint: See the specification section 6.25. "Validation keywords - type"
+<http://json-schema.org/latest/json-schema-validation.html#rfc.section.6.25>
+```
 
-Install dependencies
+or
 
-    mix deps.get
+```
+--- UNRESOLVED REFERENCE ----------------------------------- all_of_example.json
 
-Compile project
 
-    mix compile
+The following reference at `#/allOf/0/color` could not be resolved
 
-and you are good to go.
+    "$ref": #/definitions/kolor
+            ^^^^^^^^^^^^^^^^^^^
 
-#### Tests
 
-Run the standard mix task
+Hint: See the specification section 9. "Base URI and dereferencing"
+<http://json-schema.org/latest/json-schema-core.html#rfc.section.9>
+```
 
-    mix test
+If you encounter an error while using `js2e` that does not mimic the above
+Elm-like style, but instead looks like an Elixir stacktrace, please report this
+as a bug by opening an issue and includin a JSON schema example that recreates
+the error.
 
-for test coverage run
+## Contributing
+
+If you feel like something is missing/wrong or if I've misinterpreted the JSON
+schema spec, feel free to open an issue so we can discuss a solution.
 
-    mix coveralls.html
+Please consult `CONTRIBUTING.md` first before opening an issue.
diff --git a/lib/js2e.ex b/lib/js2e.ex
@@ -110,6 +110,7 @@ defmodule JS2E do
   def generate(schema_paths, module_name) do
     Logger.info("Parsing JSON schema files!")
     parser_result = Parser.parse_schema_files(schema_paths)
+
     pretty_parser_warnings(parser_result.warnings)
 
     if length(parser_result.errors) > 0 do

diff --git a/lib/parser/all_of_parser.ex b/lib/parser/all_of_parser.ex
@@ -30,7 +30,7 @@ defmodule JS2E.Parser.AllOfParser do
   """
 
   require Logger
-  alias JS2E.Parser.{Util, ErrorUtil, ParserResult}
+  alias JS2E.Parser.{Util, ParserResult}
   alias JS2E.{Types, TypePath}
   alias JS2E.Types.AllOfType
 
@@ -50,7 +50,7 @@ defmodule JS2E.Parser.AllOfParser do
 
   """
   @impl JS2E.Parser.ParserBehaviour
-  @spec type?(Types.node()) :: boolean
+  @spec type?(Types.schemaNode()) :: boolean
   def type?(%{"allOf" => all_of})
       when is_list(all_of) and length(all_of) > 0,
       do: true
@@ -61,8 +61,13 @@ defmodule JS2E.Parser.AllOfParser do
   Parses a JSON schema allOf type into an `JS2E.Types.AllOfType`.
   """
   @impl JS2E.Parser.ParserBehaviour
-  @spec parse(Types.node(), URI.t(), URI.t() | nil, TypePath.t(), String.t()) ::
-          ParserResult.t()
+  @spec parse(
+          Types.schemaNode(),
+          URI.t(),
+          URI.t() | nil,
+          TypePath.t(),
+          String.t()
+        ) :: ParserResult.t()
   def parse(%{"allOf" => all_of}, parent_id, id, path, name)
       when is_list(all_of) do
     child_path = TypePath.add_child(path, "allOf")

diff --git a/lib/parser/error_util.ex b/lib/parser/error_util.ex
@@ -128,8 +128,11 @@ defmodule JS2E.Parser.ErrorUtil do
     ParserError.new(identifier, :invalid_uri, error_msg)
   end
 
-  @spec unknown_node_type(Types.typeIdentifier(), String.t(), Types.node()) ::
-          ParserError.t()
+  @spec unknown_node_type(
+          Types.typeIdentifier(),
+          String.t(),
+          Types.schemaNode()
+        ) :: ParserError.t()
   def unknown_node_type(identifier, name, schema_node) do
     full_identifier =
       identifier
@@ -178,12 +181,12 @@ defmodule JS2E.Parser.ErrorUtil do
     end
   end
 
-  @spec error_markings(String.t()) :: String.t()
+  @spec error_markings(String.t()) :: [String.t()]
   defp error_markings(value) do
     red(String.duplicate("^", String.length(value)))
   end
 
-  @spec red(String.t()) :: list
+  @spec red(String.t()) :: [String.t()]
   defp red(str) do
     IO.ANSI.format([:red, str])
   end

diff --git a/lib/parser/one_of_parser.ex b/lib/parser/one_of_parser.ex
@@ -50,7 +50,7 @@ defmodule JS2E.Parser.OneOfParser do
 
   """
   @impl JS2E.Parser.ParserBehaviour
-  @spec type?(Types.node()) :: boolean
+  @spec type?(Types.schemaNode()) :: boolean
   def type?(schema_node) do
     one_of = schema_node["oneOf"]
     is_list(one_of) && length(one_of) > 0
@@ -60,7 +60,7 @@ defmodule JS2E.Parser.OneOfParser do
   Parses a JSON schema oneOf type into an `JS2E.Types.OneOfType`.
   """
   @impl JS2E.Parser.ParserBehaviour
-  @spec parse(Types.node(), URI.t(), URI.t(), TypePath.t(), String.t()) ::
+  @spec parse(Types.schemaNode(), URI.t(), URI.t(), TypePath.t(), String.t()) ::
           ParserResult.t()
   def parse(%{"oneOf" => one_of}, parent_id, id, path, name)
       when is_list(one_of) do

diff --git a/lib/parser/parser.ex b/lib/parser/parser.ex
@@ -5,9 +5,7 @@ defmodule JS2E.Parser do
   """
 
   require Logger
-  alias JS2E.Types
   alias JS2E.Parser.{RootParser, SchemaResult, ErrorUtil}
-  alias JS2E.Printers.Util
 
   @spec parse_schema_files([Path.t()]) :: SchemaResult.t()
   def parse_schema_files(schema_paths) do

diff --git a/lib/parser/parser_result_types.ex b/lib/parser/parser_result_types.ex
@@ -95,7 +95,7 @@ defmodule JS2E.Parser.ParserResult do
   type dictionaries to the list of errors in the merged `ParserResult`.
 
   """
-  @spec merge(ParserResult.t(), ParserResult.t()) :: ParserResult.t()
+  @spec merge(t, t) :: t
   def merge(
         %__MODULE__{
           type_dict: type_dict1,
@@ -141,8 +141,8 @@ defmodule JS2E.Parser.SchemaResult do
 
   @type t :: %__MODULE__{
           schema_dict: Types.schemaDictionary(),
-          warnings: [{Path.t(), ParserWarning.t()}],
-          errors: [{Path.t(), ParserError.t()}]
+          warnings: [{Path.t(), [ParserWarning.t()]}],
+          errors: [{Path.t(), [ParserError.t()]}]
         }
 
   defstruct [:schema_dict, :warnings, :errors]
@@ -158,13 +158,9 @@ defmodule JS2E.Parser.SchemaResult do
   dictionary corresponding to the succesfully parsed JSON schema files,
   and a list of warnings and errors encountered while parsing.
   """
-  @spec new(
-          [{Path.t(), Types.schemaDictionary()}],
-          [{Path.t(), ParserWarning.t()}],
-          [
-            {Path.t(), ParserError.t()}
-          ]
-        ) :: t
+  @spec new(Types.schemaDictionary(), [{Path.t(), [ParserWarning.t()]}], [
+          {Path.t(), [ParserError.t()]}
+        ]) :: t
   def new(schema_dict, warnings \\ [], errors \\ []) do
     %__MODULE__{schema_dict: schema_dict, warnings: warnings, errors: errors}
   end
@@ -174,7 +170,7 @@ defmodule JS2E.Parser.SchemaResult do
   schema dictionaries to the list of errors in the merged `SchemaResult`.
 
   """
-  @spec merge(SchemaResult.t(), SchemaResult.t()) :: SchemaResult.t()
+  @spec merge(t, t) :: t
   def merge(
         %__MODULE__{
           schema_dict: schema_dict1,

diff --git a/lib/parser/root_parser.ex b/lib/parser/root_parser.ex
@@ -6,25 +6,29 @@ defmodule JS2E.Parser.RootParser do
   require Logger
 
   alias JS2E.Parser.{
+    AllOfParser,
+    AnyOfParser,
     ArrayParser,
     DefinitionsParser,
     ObjectParser,
+    OneOfParser,
     TupleParser,
     TypeReferenceParser,
     Util,
     ErrorUtil,
+    ParserError,
     ParserResult,
     SchemaResult
   }
 
   alias JS2E.{TypePath, Types}
   alias JS2E.Types.SchemaDefinition
 
-  @spec parse_schema(Types.schemaNode(), String.t()) :: SchemaResult.t()
+  @spec parse_schema(Types.schemaNode(), Path.t()) :: SchemaResult.t()
   def parse_schema(root_node, schema_file_path) do
     with {:ok, _schema_version} <- parse_schema_version(root_node),
          {:ok, schema_id} <- parse_schema_id(root_node) do
-      title = Map.get(root_node, "title", "")
+      title = Map.get(root_node, "title", "Root")
       description = Map.get(root_node, "description")
 
       root_node_no_def = Map.delete(root_node, "definitions")
@@ -89,22 +93,33 @@ defmodule JS2E.Parser.RootParser do
   end
 
   @spec parse_root_object(map, URI.t(), String.t()) :: ParserResult.t()
-  defp parse_root_object(schema_root_node, schema_id, _title) do
+  defp parse_root_object(schema_root_node, schema_id, name) do
     type_path = TypePath.from_string("#")
-    name = "#"
 
     cond do
+      AllOfParser.type?(schema_root_node) ->
+        schema_root_node
+        |> AllOfParser.parse(schema_root_node, schema_id, type_path, name)
+
+      AnyOfParser.type?(schema_root_node) ->
+        schema_root_node
+        |> AnyOfParser.parse(schema_root_node, schema_id, type_path, name)
+
       ArrayParser.type?(schema_root_node) ->
         schema_root_node
-        |> Util.parse_type(schema_id, [], name)
+        |> ArrayParser.parse(schema_root_node, schema_id, type_path, name)
 
       ObjectParser.type?(schema_root_node) ->
         schema_root_node
-        |> Util.parse_type(schema_id, [], name)
+        |> ObjectParser.parse(schema_root_node, schema_id, type_path, name)
+
+      OneOfParser.type?(schema_root_node) ->
+        schema_root_node
+        |> OneOfParser.parse(schema_root_node, schema_id, type_path, name)
 
       TupleParser.type?(schema_root_node) ->
         schema_root_node
-        |> Util.parse_type(schema_id, [], name)
+        |> TupleParser.parse(schema_root_node, schema_id, type_path, name)
 
       TypeReferenceParser.type?(schema_root_node) ->
         schema_root_node

diff --git a/lib/parser/tuple_parser.ex b/lib/parser/tuple_parser.ex
@@ -34,7 +34,7 @@ defmodule JS2E.Parser.TupleParser do
 
   """
   @impl JS2E.Parser.ParserBehaviour
-  @spec type?(Types.node()) :: boolean
+  @spec type?(Types.schemaNode()) :: boolean
   def type?(schema_node) do
     items = schema_node["items"]
     is_list(items)
@@ -44,8 +44,13 @@ defmodule JS2E.Parser.TupleParser do
   Parses a JSON schema array type into an `JS2E.Types.TupleType`.
   """
   @impl JS2E.Parser.ParserBehaviour
-  @spec parse(Types.node(), URI.t(), URI.t() | nil, TypePath.t(), String.t()) ::
-          ParserResult.t()
+  @spec parse(
+          Types.schemaNode(),
+          URI.t(),
+          URI.t() | nil,
+          TypePath.t(),
+          String.t()
+        ) :: ParserResult.t()
   def parse(%{"items" => items}, parent_id, id, path, name)
       when is_list(items) do
     child_path = TypePath.add_child(path, "items")
@@ -65,10 +70,4 @@ defmodule JS2E.Parser.TupleParser do
     |> ParserResult.new()
     |> ParserResult.merge(child_types_result)
   end
-
-  def parse(%{"items" => items}, _parent_id, _id, path, _name) do
-    items_type = ErrorUtil.get_type(items)
-    error = ErrorUtil.invalid_type(path, "items", "list or object", items_type)
-    ParserResult.new(%{}, [], [error])
-  end
 end