Include support for multiple reporter output types, solves #109

[xanish]

Hey,

I was learning Go and thought I'd try contributing to this repo. This merge solves the issue #109.

I have kept the input param as comma separated string since that was used on existing params like exclude-file-types.

I wasn't sure if we would need validation to prevent running in case reporters were passed as JSON and Standard but no output file was specified. Maybe we should add a validation to force output to be present so that it can write JSON to the file and still output to Stdout.

Do let me know in case I've missed something or any change is needed.

[xanish]

hey @ccoVeille is it good to merge?

[ccoVeille]

Yes, that's why I approved 😁

[kehoecj]

@xanish Combining multiple reporters with --output results in confusing behavior as it's not clear as a user what will be produced in the output file. With multiple reporters we need to rethink how we handle outputting the reporters to a file. I like the way chef inspec handles multiple reporters where an output file is declared with the reporter type.

[xanish]

@kehoecj i can implement it like chef inspec. Do you also want me to get rid of the file flag and then include file in the reporter flag separated by ":".

Will have to use flag.Var to get multiple --reporter as an array then process it.

Flags would be as follows --reporter json:some_file_path --reporter junit:another_file_path

[kehoecj]

@kehoecj i can implement it like chef inspec. Do you also want me to get rid of the file flag and then include file in the reporter flag separated by ":".

Will have to use flag.Var to get multiple --reporter as an array then process it.

Flags would be as follows --reporter json:some_file_path --reporter junit:another_file_path

Yes, that sounds great. So to summarize:

1. Remove --output flag
2. Allowed multiple reporters to be specified by multiple --reporter flags
3. Optionally allow users to specify a file for each reporter by adding the file path and name after the reporter type
4. Document this on the README

[xanish]

Done, lemme know if it is as expected 😄

[kehoecj]

Done, lemme know if it is as expected 😄

Very nice! One last thing, I'm not sure the report should display to stdout if an output file is specified. What do you think?

[xanish]

Hey @kehoecj so i was working on making standard reporter output to file as well since that would make things "standard" across all reporters. But I had one doubt currently all reporters when passed an output file print to stdout as well as output to file do we want to make sure it only does either of it? Like if output is specified then only write to file otherwise write to stdout.

[kehoecj]

Hey @kehoecj so i was working on making standard reporter output to file as well since that would make things "standard" across all reporters. But I had one doubt currently all reporters when passed an output file print to stdout as well as output to file do we want to make sure it only does either of it? Like if output is specified then only write to file otherwise write to stdout.

Yes, let's go ahead and standardize it:

• If an output file is passed it will always send the results to the output file
• If they just want stdout they just pass the reporter name, i.e. --reporter json
• If they want to do both they'd pass two reporters: --reporter json --reporter json:/path/to/file.json

@xanish What do you think?

[xanish]

Ok so i made the changes and also merged main from upstream. I have a doubt regarding how the reporter flag will work with ENV, while testing i noticed that passing something like

CFV_REPORTER=standard:output.txt go run cmd/validator/validator.go --reporter=junit:op.xml test/fixtures/good.csv

uses both the env value and cli value

EDIT:

Ignore figured out the issue, flag.Parse() should happen before the env check and set logic

[kehoecj]

Hey @kehoecj can you help me out with this, I'm not seeing what the exact error is from the linter.

  Running [/home/runner/golangci-lint-1.57.2-linux-amd64/golangci-lint run --out-format=github-actions --timeout=10m] in [] ...
  Error: File is not `gci`-ed with --skip-generated -s standard -s default -s prefix(github.com/Boeing/config-file-validator) (gci)
  Error: File is not `gci`-ed with --skip-generated -s standard -s default -s prefix(github.com/Boeing/config-file-validator) (gci)
  
  Error: issues found
  Ran golangci-lint in 7903ms

Did you try golangci-lint run --fix?

[xanish]

Im kindof getting conflicting error with my local machine saying

danee@TOASTER:~/Code/open-source/config-file-validator> golangci-lint run --fix
pkg/reporter/reporter_test.go:507:3: comment-spacings: no space between comment delimiter and comment text (revive)
		//nolint: testifylint // in this use case it's ok to use assert.Error

and github action saying that it does not need space 😅

  Running [/home/runner/golangci-lint-1.57.2-linux-amd64/golangci-lint run --out-format=github-actions --timeout=10m] in [] ...
  Error: directive `// nolint: testifylint // in this use case it's ok to use assert.Error` should be written without leading space as `//nolint: testifylint // in this use case it's ok to use assert.Error` (nolintlint)

[kehoecj]

Im kindof getting conflicting error with my local machine saying

danee@TOASTER:~/Code/open-source/config-file-validator> golangci-lint run --fix
pkg/reporter/reporter_test.go:507:3: comment-spacings: no space between comment delimiter and comment text (revive)
		//nolint: testifylint // in this use case it's ok to use assert.Error

and github action saying that it does not need space 😅

  Running [/home/runner/golangci-lint-1.57.2-linux-amd64/golangci-lint run --out-format=github-actions --timeout=10m] in [] ...
  Error: directive `// nolint: testifylint // in this use case it's ok to use assert.Error` should be written without leading space as `//nolint: testifylint // in this use case it's ok to use assert.Error` (nolintlint)

@ccoVeille Any ideas/suggestions?

[ccoVeille]

Im kindof getting conflicting error with my local machine saying

danee@TOASTER:~/Code/open-source/config-file-validator> golangci-lint run --fix
pkg/reporter/reporter_test.go:507:3: comment-spacings: no space between comment delimiter and comment text (revive)
		//nolint: testifylint // in this use case it's ok to use assert.Error

and github action saying that it does not need space 😅

  Running [/home/runner/golangci-lint-1.57.2-linux-amd64/golangci-lint run --out-format=github-actions --timeout=10m] in [] ...
  Error: directive `// nolint: testifylint // in this use case it's ok to use assert.Error` should be written without leading space as `//nolint: testifylint // in this use case it's ok to use assert.Error` (nolintlint)

Here is what is expected I think

#181 (comment)

cc @kehoecj

[kehoecj]

The output for the standard reporter no longer colorizes the output or formats it properly:

    ✓ /tests/fixtures/exclude-file-types/excluded.json    ✓ /tests/fixtures/exclude-file-types/not-excluded.toml    ✓ /tests/fixtures/good.csv    ✓ /tests/fixtures/good.editorconfig    ✓ /tests/fixtures/good.env    ✓ /tests/fixtures/good.hcl    ✓ /tests/fixtures/good.hocon    ✓ /tests/fixtures/good.ini    ✓ /tests/fixtures/good.json    ✓ /tests/fixtures/good.plist    ✓ /tests/fixtures/good.properties    ✓ /tests/fixtures/good.toml    ✓ /tests/fixtures/good.yaml    ✓ /tests/fixtures/mixedcase-extension/good.CSv    ✓ /tests/fixtures/mixedcase-extension/good.HCl    ✓ /tests/fixtures/mixedcase-extension/good.INi    ✓ /tests/fixtures/mixedcase-extension/good.JSon    ✓ /tests/fixtures/mixedcase-extension/good.PList    ✓ /tests/fixtures/mixedcase-extension/good.PRoperties    ✓ /tests/fixtures/mixedcase-extension/good.TOml    ✓ /tests/fixtures/mixedcase-extension/good.YAml    × /tests/fixtures/subdir/bad.json
        error: Error at line 3 column 14: invalid character ':' after array element
    × /tests/fixtures/subdir/bad.toml
        error: Error at line 6 column 6: toml: expected character =
    × /tests/fixtures/subdir/bad.yml
        error: yaml: line 3: did not find expected '-' indicator
    ✓ /tests/fixtures/subdir/good.json    × /tests/fixtures/subdir2/bad.csv
        error: parse error on line 1, column 20: bare " in non-quoted-field
    × /tests/fixtures/subdir2/bad.editorconfig
        error: cannot load ini file: unclosed section: [*.md
    × /tests/fixtures/subdir2/bad.env
        error: Error at line 2: invalid escape sequence: "a"
    × /tests/fixtures/subdir2/bad.hcl
        error: error at line 1 column 1: :1,1-2: Invalid argument name; Argument names must not be quoted.
    × /tests/fixtures/subdir2/bad.hocon
        error: two adjacent commas at: 3:16, adjacent commas in arrays and objects are invalid!
    × /tests/fixtures/subdir2/bad.ini
        error: key-value delimiter not found: name value
    × /tests/fixtures/subdir2/bad.json
        error: Error at line 1 column 3: invalid character '}' looking for beginning of value
    × /tests/fixtures/subdir2/bad.plist
        error: plist: error parsing XML property list: missing value in dictionary
    × /tests/fixtures/subdir2/bad.properties
        error: circular reference in:
               key=${key}
    ✓ /tests/fixtures/subdir2/good.ini    ✓ /tests/fixtures/subdir2/good.json    ✓ /tests/fixtures/uppercase-extension/good.CSV    ✓ /tests/fixtures/uppercase-extension/good.HCL    ✓ /tests/fixtures/uppercase-extension/good.INI    ✓ /tests/fixtures/uppercase-extension/good.JSON    ✓ /tests/fixtures/uppercase-extension/good.PLIST    ✓ /tests/fixtures/uppercase-extension/good.PROPERTIES    ✓ /tests/fixtures/uppercase-extension/good.TOML    ✓ /tests/fixtures/uppercase-extension/good.YAML    ✓ /tests/fixtures/with-depth/additional-depth/sample.json    ✓ /tests/fixtures/with-depth/sample.json    ✓ /tests/output/example/good.json    ✓ /tests/output/example/result.json    ✓ /tests/output/example/result.xml    ✓ /tests/report.json    ✓ /tests/report.xml    ✓ /tests/scan.json    ✓ /tests/scan.xml%   

[xanish]

weird it works for me locally, did you merge main? i guess something must've broken.

[xanish]

@kehoecj can you check now

Also lemme know how do you want me to resolve the indentation thing

[kehoecj]

Final checks passed this morning. Thank you for the PR @xanish !