feat: add json formatter

[Kashugoyal]

Resolves https://github.com/Boeing/config-file-validator/issues/159

This PR:

1. Adds a JSON formatter to format JSON files
   • Uses the package https://github.com/tidwall/pretty/tree/master for JSON formatting
2. Adds the CLI and env flags for enable the formatter
3. Adds and updates relevant unit and functional tests
4. Updates existing JSON files in the repository to match the format

Left for Future work:

1. Support for formatting check of other file types
2. Allow configuration for formatters. For example allow single vs double indent

[kehoecj]

@Kashugoyal Thanks for the PR - please address all Golang-CI comments and fix failing unit tests

[Kashugoyal]

@Kashugoyal Thanks for the PR - please address all Golang-CI comments and fix failing unit tests

@kehoecj PR is ready for review.

[Kashugoyal]

@ccoVeille PR is ready for another look

[ccoVeille]

Instead of reviewing the PR, I have just shared my thoughts and concerns on the issue that this PR is about.

https://github.com/Boeing/config-file-validator/issues/159#issuecomment-3418639739

Your code is OK. I could review it. But for now, I prefered to step out.

Note to make it clear I'm not against what you coded or the way you coded, I just feel it shouldn't be added to the tool

[kehoecj]

@Kashugoyal Please make the following changes to the implementation based off the discussion in #159 :

1. Change the CLI --format flag to --check-format to be more explicit that it will simply validate the formatting against the defaults, not make any changes. Users should be able to scope this to list of config types, i.e. --check-format=json,yaml
2. Passing --check-format should only validate the formatting, not make the formatting changes. That will be handled by a --fix flag that will be implemented later
3. Add new code under pkg/validator to validate the formatting against the defaults if --check-format is passed

Let me know if that makes sense.

[Kashugoyal]

@Kashugoyal Please make the following changes to the implementation based off the discussion in #159 :

1. Change the CLI --format flag to --check-format to be more explicit that it will simply validate the formatting against the defaults, not make any changes. Users should be able to scope this to list of config types, i.e. --check-format=json,yaml
2. Passing --check-format should only validate the formatting, not make the formatting changes. That will be handled by a --fix flag that will be implemented later
3. Add new code under pkg/validator to validate the formatting against the defaults if --check-format is passed

Let me know if that makes sense.

@kehoecj what design do you prefer? I can list a few options below:

1. Add another method to the Validator interface.

   type Validator interface {
        Validate(b []byte) (bool, error)
        ValidateFormat(b []byte, options interface{}) (bool, error)
   }
   

   options is a future looking feature. Can be skipped for the first iteration.

2. Add a parameter to the existing method

   type Validator interface {
        Validate(b []byte, checkFormat bool) (bool, error)
   }
   

[kehoecj]

@Kashugoyal Please make the following changes to the implementation based off the discussion in #159 :

1. Change the CLI --format flag to --check-format to be more explicit that it will simply validate the formatting against the defaults, not make any changes. Users should be able to scope this to list of config types, i.e. --check-format=json,yaml
2. Passing --check-format should only validate the formatting, not make the formatting changes. That will be handled by a --fix flag that will be implemented later
3. Add new code under pkg/validator to validate the formatting against the defaults if --check-format is passed

Let me know if that makes sense.

@kehoecj what design do you prefer? I can list a few options below:

1. Add another method to the Validator interface.

   type Validator interface {
        Validate(b []byte) (bool, error)
        ValidateFormat(b []byte, options interface{}) (bool, error)
   }
   

   options is a future looking feature. Can be skipped for the first iteration.

2. Add a parameter to the existing method

   type Validator interface {
        Validate(b []byte, checkFormat bool) (bool, error)
   }
   

Good timing, I am going through the same design decision now that I'm adding schema. I was leaning toward #1 but the downside is that the interface is now more complicated. I don't like #2 because it makes the interface definition more complicated and forces design decisions into the implementing functions.

I think I'm leaning toward interface composition:

type SyntaxValidator interface {
  ValidateSyntax(b []byte) (bool, error)
}

type FormatValidator interface {
  ValidateFormat(Validate(b []byte, options interface{}) (bool, error)
}

// coming in my Sarif PR
type SchemaValidator interface {
  ValidateSchema(Validate(b []byte, schemaDefinition interface{}) (bool, error)
}

type Validator interface {
  SyntaxValidator
  FormatValidator
  SchemaValidator
}

That way, when required, the different validator interfaces can be fulfilled and referenced. What do you think?

[Kashugoyal]

@Kashugoyal Please make the following changes to the implementation based off the discussion in #159 :

1. Change the CLI --format flag to --check-format to be more explicit that it will simply validate the formatting against the defaults, not make any changes. Users should be able to scope this to list of config types, i.e. --check-format=json,yaml
2. Passing --check-format should only validate the formatting, not make the formatting changes. That will be handled by a --fix flag that will be implemented later
3. Add new code under pkg/validator to validate the formatting against the defaults if --check-format is passed

Let me know if that makes sense.

@kehoecj what design do you prefer? I can list a few options below:

1. Add another method to the Validator interface.

   type Validator interface {
        Validate(b []byte) (bool, error)
        ValidateFormat(b []byte, options interface{}) (bool, error)
   }
   

   options is a future looking feature. Can be skipped for the first iteration.

2. Add a parameter to the existing method

   type Validator interface {
        Validate(b []byte, checkFormat bool) (bool, error)
   }
   

Good timing, I am going through the same design decision now that I'm adding schema. I was leaning toward #1 but the downside is that the interface is now more complicated. I don't like #2 because it makes the interface definition more complicated and forces design decisions into the implementing functions.

I think I'm leaning toward interface composition:

type SyntaxValidator interface {
  ValidateSyntax(b []byte) (bool, error)
}

type FormatValidator interface {
  ValidateFormat(Validate(b []byte, options interface{}) (bool, error)
}

// coming in my Sarif PR
type SchemaValidator interface {
  ValidateSchema(Validate(b []byte, schemaDefinition interface{}) (bool, error)
}

type Validator interface {
  SyntaxValidator
  FormatValidator
  SchemaValidator
}

That way, when required, the different validator interfaces can be fulfilled and referenced. What do you think?

I like your idea. Will do that!

[Kashugoyal]

@kehoecj marking the MR ready for review. It has been a lot of back and forth therefore I may have missed some things in my MR, please review 😄

[Kashugoyal]

During functional testing the following issue was found:

When passing the --check-format, the provided path is not honored by the confi-file-validator tool and it instead scans the current working directory:

~/src/github.com/boeing/pr/formatting/config-file-validator$ ./validator --check-for
mat test/fixtures/bad_format.json 
    ✓ /src/github.com/boeing/pr/formatting/config-file-validator/.github/dependabot.yml
    ✓ /src/github.com/boeing/pr/formatting/config-file-validator/.github/workflows/changelog.yml
    ✓ /src/github.com/boeing/pr/formatting/config-file-validator/.github/workflows/go.yml
    ✓ /src/github.com/boeing/pr/formatting/config-file-validator/.github/workflows/golangci-lint.yml
    ✓ /src/github.com/boeing/pr/formatting/config-file-validator/.github/workflows/goreportcard.yaml
    ✓ /src/github.com/boeing/pr/formatting/config-file-validator/.github/workflows/mega-linter.yml
    ✓ /src/github.com/boeing/pr/formatting/config-file-validator/.github/workflows/release.yml
    ✓ /src/github.com/boeing/pr/formatting/config-file-validator/.github/workflows/scorecard.yml

When I remove the --check-format flag, the tools works as expected

In the above case, the check-format flag assumes the following string is the file type and not the path. I added a validation function for this case and also added a test case for this scenario.