Feature: Store hank output in json file #184

naman070 · 2022-06-03T20:05:01Z

Provided an argument based option where if command is run like:
rebar3 hank -o "output.json"
then it stores the generated output of hank in the json file.
This json file can further be used for better analysis of dead code.

Sample Json:

[
  {
    "hank_rule_broken": "unnecessary_function_arguments",
    "message": "hello_world/2 doesn't need its #2 argument",
    "path": "apps/library/src/library_app.erl",
    "start_line": 19,
    "title": "Unused function arguments"
  },
  {
    "hank_rule_broken": "unused_hrls",
    "message": "This file is unused",
    "path": "apps/library/src/demo_var.hrl",
    "start_line": 0,
    "title": "Unused hrl files"
  }
]

CLAassistant · 2022-06-03T20:05:06Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ elbrujohalcon
❌ Naman Gupta

Naman Gupta seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

rebar.config

src/rebar3_hank_prv.erl

elbrujohalcon

This is a good idea, but (besides the changes that I suggest in the current code), what I would like to see is…

That this would also be available as a rebar.config option with then hank section.
Documentation about this added to the README.md.
Some tests that ensure that we don't go down on code coverage.

Thanks :)

src/rebar3_hank_prv.erl

elbrujohalcon

A few grammatical/syntactical/consistency-related amendments.

elbrujohalcon · 2022-06-08T08:21:47Z

README.md

+  Usage: rebar3 hank [-u <unused_ignores>] [-o <output_json_file>]
+
+  -u, --unused_ignores    Warn on unused ignores (default: true).
+  -o, --output_json_file  Emit output (in JSON format) to a file (default: empty string - meaning: donot emit output


Suggested change

-o, --output_json_file Emit output (in JSON format) to a file (default: empty string - meaning: donot emit output

-o, --output_json_file Emit output (in JSON format) to a file (default: empty string - meaning: do not emit output).

elbrujohalcon · 2022-06-08T08:22:30Z

README.md


 ```
 By default, Hank will emit warnings such as the following ones if you are ignoring rules that you don't need to ignore (more on that below). But you can turn those warnings off, by using `--unused_ignores=no`.

 It's worth noting that, even when those warnings are printed, that doesn't affect the overall result of the command. That is, if Hank can't find any instances of oxbow code, it will return successfully (i.e. `exit code: 0`) even when it may print these warnings.

+By default, if hank detect issues in your code, it will print those issue on the console. But you can save this result in JSON format,


Suggested change

By default, if hank detect issues in your code, it will print those issue on the console. But you can save this result in JSON format,

By default, if Hank detects issues in your code, it will print those issues on the console. But you can save this result in a file (in JSON format),

elbrujohalcon · 2022-06-08T08:23:52Z

src/hank_rule.erl

+        unused_callbacks ->
+            <<"Unused callback functions">>;
+        unnecessary_function_arguments ->
+            <<"Unused function arguments found">>;


Suggested change

<<"Unused function arguments found">>;

<<"Unused function arguments">>;

elbrujohalcon · 2022-06-08T08:24:08Z

src/hank_rule.erl

+        unnecessary_function_arguments ->
+            <<"Unused function arguments found">>;
+        single_use_hrls ->
+            <<"Hrl is only used once">>


Suggested change

<<"Hrl is only used once">>

<<"Hrl file only used once">>

elbrujohalcon · 2022-06-08T08:24:22Z

src/hank_rule.erl

+        unused_macros ->
+            <<"Unused Macros">>;
+        single_use_hrl_attrs ->
+            <<"Macro is only used once">>;


Suggested change

<<"Macro is only used once">>;

<<"Macro only used once">>;

elbrujohalcon · 2022-06-08T08:24:34Z

src/hank_rule.erl

+        single_use_hrl_attrs ->
+            <<"Macro is only used once">>;
+        unused_record_fields ->
+            <<"Field in the record is unused">>;


Suggested change

<<"Field in the record is unused">>;

<<"Unused field in record">>;

elbrujohalcon · 2022-06-08T08:24:54Z

src/rebar3_hank_prv.erl

+        $o,
+        "output_json_file",
+        string,
+        "Emit output (in JSON format) to a file (default: empty string - meaning: donot emit output"}


Suggested change

"Emit output (in JSON format) to a file (default: empty string - meaning: donot emit output"}

"Emit output (in JSON format) to a file (default: empty string - meaning: do not emit output)"}

elbrujohalcon · 2022-06-08T08:27:54Z

@naman070 You should run rebar3 test on your code :)

So, at this stage, what's still missing is:

To make this option also available as a rebar.config option within the hank section.
Some tests that ensure that we don't go down on code coverage.

Thanks again :)

naman070 · 2022-06-08T11:50:00Z

@elbrujohalcon
What do you mean by your 1st point => "To make this option also available as a rebar.config option within the hank section.". Can you please elaborate more.

elbrujohalcon · 2022-06-08T12:35:03Z

@elbrujohalcon What do you mean by your 1st point => "To make this option also available as a rebar.config option within the hank section.". Can you please elaborate more.

Yeah, I want users to be able to include this in their rebar.config…

{hank, [{output_json_file, "/the/file/to/put/the/output.json"}]}.

And have Hank pick it up every time they run rebar3 hank on the project.
Just like they can do now with ignore or parsing_style.

naman070 · 2022-06-08T14:16:18Z

@elbrujohalcon

Have added changes so that it can be added as an option in rebar.config.
Testcases are still pending (Should I create a separate file for testcases or should I incorporate in some existing file)

pbrudnick

Hey nice work, thank you @naman070 !
I'd expect a json output example here in the PR description to review it. I mean, what's the desired output?

Also I agree with the tests request.

naman070 · 2022-06-08T14:45:28Z

@elbrujohalcon Does the output of rebar3 test supposed to look like this?

===> Plugin jsx does not export init/1. It will not be used.
===> Verifying dependencies...
===> Analyzing applications...
===> Compiling rebar3_hank
===> Scissors cuts paper, paper covers rock, rock crushes lizard, lizard poisons Spock, Spock smashes scissors, scissors decapitates lizard, lizard eats paper, paper disproves Spock, Spock vaporizes rock, and as it always has, rock crushes scissors.
src/rebar3_hank_prv.erl:43: The word "JSON" in string is unknown. Maybe you wanted to use "jason" or "sion" or "slon" or "soln" or "son" or "sond" or "sone" or "song" or "sonk" or "sons" or "soon" or "sorn" or "sown" or "jeon" or "joan" or "john" or "join" or "Tjon"?

elbrujohalcon · 2022-06-08T14:54:10Z

@elbrujohalcon Does the output of rebar3 test supposed to look like this?


===> Plugin jsx does not export init/1. It will not be used.

===> Verifying dependencies...

===> Analyzing applications...

===> Compiling rebar3_hank

===> Scissors cuts paper, paper covers rock, rock crushes lizard, lizard poisons Spock, Spock smashes scissors, scissors decapitates lizard, lizard eats paper, paper disproves Spock, Spock vaporizes rock, and as it always has, rock crushes scissors.

src/rebar3_hank_prv.erl:43: The word "JSON" in string is unknown. Maybe you wanted to use "jason" or "sion" or "slon" or "soln" or "son" or "sond" or "sone" or "song" or "sonk" or "sons" or "soon" or "sorn" or "sown" or "jeon" or "joan" or "john" or "join" or "Tjon"?

It's telling you that Sheldon (the spell checker) doesn't know the word "JSON".
Add it to nextroll.dict and you will be able to move on... ;)

elbrujohalcon · 2022-06-08T14:59:02Z

src/rebar3_hank_prv.erl

+    ConvertedResult = convert_data_to_binary(Result),
+    EncodedResult = jsx:encode(ConvertedResult),


It's not terribly wrong, but I would prefer to only do this if we're going to use it (i.e. First try to come up with the name of the output file, if it's not empty,... Only then convert, encode, and emit the output).

And please document the rebar.config option in the README.md

elbrujohalcon · 2022-06-08T15:01:34Z

src/rebar3_hank_prv.erl

+            end
+    end.
+
+-spec convert_data_to_binary([hank_rules:result()]) -> list().


This function doesn't return binaries. Therefore, its name is wrong. A better name would be results_to_json.

Feature: Store hank output in json format

1c9c875

paulo-ferraz-oliveira reviewed Jun 4, 2022

View reviewed changes

rebar.config Outdated Show resolved Hide resolved

paulo-ferraz-oliveira reviewed Jun 4, 2022

View reviewed changes

src/rebar3_hank_prv.erl Outdated Show resolved Hide resolved

paulo-ferraz-oliveira reviewed Jun 4, 2022

View reviewed changes

src/rebar3_hank_prv.erl Outdated Show resolved Hide resolved

NMN070 and others added 3 commits June 5, 2022 22:35

Code review changes

3666eb7

CR changes

1f547d8

Merge branch 'main' into main

eea6beb

elbrujohalcon requested changes Jun 6, 2022

View reviewed changes

src/rebar3_hank_prv.erl Outdated Show resolved Hide resolved

src/rebar3_hank_prv.erl Outdated Show resolved Hide resolved

src/rebar3_hank_prv.erl Outdated Show resolved Hide resolved

src/rebar3_hank_prv.erl Outdated Show resolved Hide resolved

NMN070 added 2 commits June 8, 2022 13:43

CR changes

8b542bd

Minor change

686c17b

elbrujohalcon requested changes Jun 8, 2022

View reviewed changes

Option so that json file can be put under hank in rebar.config

6569555

Code review changes

e529329

pbrudnick reviewed Jun 8, 2022

View reviewed changes

elbrujohalcon requested changes Jun 8, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: Store hank output in json file #184

Feature: Store hank output in json file #184

naman070 commented Jun 3, 2022 •

edited

Loading

CLAassistant commented Jun 3, 2022 •

edited

Loading

elbrujohalcon left a comment

elbrujohalcon left a comment

elbrujohalcon Jun 8, 2022

elbrujohalcon Jun 8, 2022

elbrujohalcon Jun 8, 2022

elbrujohalcon Jun 8, 2022

elbrujohalcon Jun 8, 2022

elbrujohalcon Jun 8, 2022

elbrujohalcon Jun 8, 2022

elbrujohalcon commented Jun 8, 2022

naman070 commented Jun 8, 2022

elbrujohalcon commented Jun 8, 2022

naman070 commented Jun 8, 2022

pbrudnick left a comment

naman070 commented Jun 8, 2022

elbrujohalcon commented Jun 8, 2022

elbrujohalcon Jun 8, 2022

elbrujohalcon Jun 8, 2022

elbrujohalcon Jun 8, 2022

	-o, --output_json_file Emit output (in JSON format) to a file (default: empty string - meaning: donot emit output
	-o, --output_json_file Emit output (in JSON format) to a file (default: empty string - meaning: do not emit output).

	By default, if hank detect issues in your code, it will print those issue on the console. But you can save this result in JSON format,
	By default, if Hank detects issues in your code, it will print those issues on the console. But you can save this result in a file (in JSON format),

	<<"Unused function arguments found">>;
	<<"Unused function arguments">>;

	<<"Field in the record is unused">>;
	<<"Unused field in record">>;

	"Emit output (in JSON format) to a file (default: empty string - meaning: donot emit output"}
	"Emit output (in JSON format) to a file (default: empty string - meaning: do not emit output)"}

		ConvertedResult = convert_data_to_binary(Result),
		EncodedResult = jsx:encode(ConvertedResult),

Feature: Store hank output in json file #184

Are you sure you want to change the base?

Feature: Store hank output in json file #184

Conversation

naman070 commented Jun 3, 2022 • edited Loading

CLAassistant commented Jun 3, 2022 • edited Loading

elbrujohalcon left a comment

Choose a reason for hiding this comment

elbrujohalcon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elbrujohalcon commented Jun 8, 2022

naman070 commented Jun 8, 2022

elbrujohalcon commented Jun 8, 2022

naman070 commented Jun 8, 2022

pbrudnick left a comment

Choose a reason for hiding this comment

naman070 commented Jun 8, 2022

elbrujohalcon commented Jun 8, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

naman070 commented Jun 3, 2022 •

edited

Loading

CLAassistant commented Jun 3, 2022 •

edited

Loading