feat: add json formatter #336

Kashugoyal · 2025-10-09T19:39:22Z

Resolves #159

This PR:

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

Left for Future work:

Support for formatters of other file types
Additional backup flag to backup the original files before overwriting
Allow configuration for formatters. For example allow single vs double indent

kehoecj · 2025-10-10T16:57:38Z

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

Kashugoyal · 2025-10-15T00:57:59Z

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

@kehoecj PR is ready for review.

kehoecj

Works as advertised but needs some architectural changes

kehoecj · 2025-10-15T17:43:47Z

cmd/validator/validator.go

 		groupOutputPtr      = flag.String("groupby", "", "Group output by filetype, directory, pass-fail. Supported for Standard and JSON reports")
 		quietPtr            = flag.Bool("quiet", false, "If quiet flag is set. It doesn't print any output to stdout.")
 		globbingPrt         = flag.Bool("globbing", false, "If globbing flag is set, check for glob patterns in the arguments.")
+		formatPtr           = flag.Bool("format", false, "Format all supported file types in place. Only json is supported currently.")


Thinking ahead to when multiple types are supported:

Users need to be able to specify which config types they want to format. All can be the default but needs to be configurable

Need to think about how we'll allow users to specify formatting settings. I know you had identified it in future work but I think it needs to be part of this PR. I don't think we'll be able to specify this on the command line, will probably have to be a config file (or multiple config files)

Can we do flags like --format-all, --format-json, --format-yaml, etc?
format-xxx to enable formatter for a specific file type,
format-all for all
format-config to follow config specification, e.g. --format-config ./formatter_config.json

For the settings specification, it can be a JSON file with top level keys for file types.
e.g.:

"formatters": ["json", "yaml", ..], "json": { "indent": 2}, "toml": {..} ...

The implementation for the config would be a larger change and maybe we can address as a separate PR.

What about passing options to -format? Example: -format=all OR -format=json,yaml,ini?

I like the settings JSON - that makes sense to me. Agree that would be part of a larger change later, just wanted to ensure the initial design didn't constrain the later implementation

kehoecj · 2025-10-15T18:01:28Z

pkg/cli/cli.go

+	if ShouldFormat {
+		if exitCode, err := formatFiles(filesToFormat); err != nil {
+			return exitCode, fmt.Errorf("unable to format files: %w", err)
+		}
+	}


The design of this could be refactored to be more efficient:

Call the formatter in the initial foundFiles loop rather than adding it to a list then looping again.

The formatFiles function logic should be moved down into the formatter rather than in the CLI

kehoecj · 2025-10-15T18:06:53Z

go.mod

 	github.com/magiconair/properties v1.8.10
 	github.com/pelletier/go-toml/v2 v2.2.4
 	github.com/stretchr/testify v1.11.1
+	github.com/tidwall/pretty v1.2.1


I do not think we should be using this package. It appears to no longer be maintained as it hasn't been updated in 3+ years. Implementation should be changed to use the standard library's Marshall Indent

I don't know if it is possible to ensure retention of the JSON field order if we parse JSON bytes and use MarshallIndent to generate formatted output. If this is not a requirement, we can use it then.

According to the JSON spec it shouldn't matter:

An object is an unordered set of name/value pairs

We can specify that order of elements will not be preserved in the documentation

As a user I would not like it if a formatter changed the order of my fields in the JSON file. But if that is alright, I don't mind switching to MarshalIndent.
I will make that change.

@Kashugoyal what would you suggest as an alternative?

Actually after doing some research I found this method called Indent in the built in golang JSON library. Although it does not explicitly guarantee order preservation but when I looked into its implementation, it is very similar to what "pretty" does.
I am going to try that instead.

That looks promising!

Kashugoyal changed the title ~~feat: add json formatter~~ draft: feat: add json formatter Oct 9, 2025

kehoecj added OSS Community Contribution Contributions from the OSS Community waiting-on-maintainer-review PR is waiting to be reviewed and functionally tested by the maintainers labels Oct 10, 2025

kehoecj self-requested a review October 10, 2025 14:27

kehoecj added pr-action-requested PR is awaiting feedback from the submitting developer and removed waiting-on-maintainer-review PR is waiting to be reviewed and functionally tested by the maintainers labels Oct 10, 2025

Kashugoyal marked this pull request as draft October 10, 2025 17:02

Kashugoyal force-pushed the json-formatter branch from 208ce3a to dcf1526 Compare October 15, 2025 00:15

feat: add json formatter

2061a41

Kashugoyal force-pushed the json-formatter branch from dcf1526 to 2061a41 Compare October 15, 2025 00:18

Merge branch 'main' into json-formatter

bd4c2ab

Kashugoyal marked this pull request as ready for review October 15, 2025 00:57

kehoecj changed the title ~~draft: feat: add json formatter~~ feat: add json formatter Oct 15, 2025

kehoecj added waiting-on-maintainer-review PR is waiting to be reviewed and functionally tested by the maintainers and removed pr-action-requested PR is awaiting feedback from the submitting developer labels Oct 15, 2025

kehoecj requested changes Oct 15, 2025

View reviewed changes

kehoecj added pr-action-requested PR is awaiting feedback from the submitting developer and removed waiting-on-maintainer-review PR is waiting to be reviewed and functionally tested by the maintainers labels Oct 15, 2025

feat: add json formatter #336

Are you sure you want to change the base?

feat: add json formatter #336

Uh oh!

Conversation

Kashugoyal commented Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kehoecj commented Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Kashugoyal commented Oct 15, 2025

Uh oh!

kehoecj left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Kashugoyal commented Oct 9, 2025 •

edited

Loading

kehoecj commented Oct 10, 2025 •

edited

Loading