Modify command line arguments

[dguido]

The command line arguments are setup differently than I would expect for Slither. I wonder what people think about changing them? I would have expected some of these changes:

• --detect which takes a comma-separated list indicating the checks to run (default: all)
• --print which takes a comma-separated list of printers to output (default: ???)
• --output which accepts a single parameter of either stdout or json (default: stdout)
• remove exclusion rules entirely
• calling slither with no arguments should print the help
• reorganize the help screen into groups of related parameters for easier reading

[kirtixs]

Personally I'd say this is more consistent with other tools.
I'm going to take a look at this tomorrow.

[benstew]

In agreement here. I would be in favor of changing following @dguido's suggestion.

It seems like one of the only open items would be around the default --print option. I don't think we need to overthink this, just utilizing the contract summary as the default should add utility. If another printer is sought, it's easy to update the parameters. Plus it will support the existing printer documentation quite seamlessly. Happy to lend a hand @redshark1802.

[kirtixs]

I've created WIP pr in #55
@benstew thanks, any input/help is appreciated

[kirtixs]

Currently json ouput prints to a file, how should the the argument for output look like?
We would have to require one additional argument for the json output filename if json was chosen.

[benstew]

@redshark1802 do you think that specifying the output filename warrants an additional required argument? Seems like we could just default to something intuitive for the time being. We can always add this functionality later based on feedback.

[benstew]

nitpick - @dguido, maybe we should also add a --version command? Keeping things consistent with a common interface.

[disconnect3d]

Yes, we should add a `--version` :).

…

On Tue, Oct 23, 2018, 12:57 AM Ben Stewart ***@***.***> wrote: nitpick - @dguido <https://github.com/dguido>, maybe we should also add a --version command? Keeping things consistent with a common interface. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#50 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AJi7Cq_teI9Dn7Y_iyp2uxw3z7Hb75nTks5unk1xgaJpZM4XyAeB> .

[kirtixs]

added --version command
@benstew some tools require an additional argument in cases where the output should be written to file instead of stdout. This would also give the user more control and no 'unexpected' files would be created.

[montyly]

I think we should keep the exclusion rules related to the severity, such as --exclude-informational.
When reviewing for a first time a codebase, you probably want to focus only on security issues

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

This issue now has a funding of 70.0 DAI (70.0 USD @ $1.0/DAI) attached to it as part of the Ethereum Community Fund via ECF Web 3.0 Infrastructure Fund fund.

• If you would like to work on this issue you can 'start work' on the Gitcoin Issue Details page.
• Want to chip in? Add your own contribution here.
• Questions? Checkout Gitcoin Help or the Gitcoin Slack
• $50,106.08 more funded OSS Work available on the Gitcoin Issue Explorer

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

Work has been started.

These users each claimed they can complete the work by 7 months, 2 weeks from now.
Please review their action plans below:

1) redshark1802 has been approved to start work.

WIP #55

Learn more on the Gitcoin Issue Details page.

[kirtixs]

I'm also in favor of leaving the --exclude-[LEVEL] switches in but what about --exclude-backdoor etc?
We still need to decide how to handle the output switch between stdout and json: should we just put the results in results.json when json output is used or additionally ask the user for an output filename?

[montyly]

I am in favor to ask the user for an output filename, and maye have results.json as default value?

I dont have a strong opinion about the exclude-detector, but I am ok if we use the same system than for detect (--exlude detector1, detector2...), as it allows a better control for the user.

Maybe we could also have --list-detectors and --list-printer to print the text description of each detector/printer?

[disconnect3d]

We could also put json on the stdout and all the other logs to stderr. This way the user could just do slither ... > output.json.

[kirtixs]

@disconnect3d I agree, imho this a very consistent way.
@montyly going to update the --exclude detector-name.. behaviour but I'm not quite sure that we need --list-{printers,detectors} commands as the values are already displayed in the help.

[montyly]

With each detector/printer you have an help information, which describes what the detector/printer does.

It could be useful to be able to print that description directly from the command line, so the users don't need to go back to the README page

[kirtixs]

I've added --list-{printers,detectors} and utilised the already available output_to_markdown method and adapted it for printers, let me know if this ok.

Also I played a bit around with json output and I think the current behaviour is just fine, i.e --json results.json instead of adding a new --output flag. This is because printers print to stdout by default and this would require a bit more work. I'm becoming more and more unsure if this should be added.

You can also ping me on slack if you want to for further discussions.

[montyly]

Hey, I merged the PR, and added some modifications on the command line (like I removed --output, to only keep --json, we will see in the future how people are using the option)

Thanks for your help and your feedback!

@mkosowsk can you validate the bounty? thx!

[kirtixs]

You're welcome, thanks.

[mkosowsk]

@redshark1802 please submit the bounty on the Gitcoin Issue Details page and I will pay out ASAP. Thanks! :)

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

Work for 70.0 DAI (70.0 USD @ $1.0/DAI) has been submitted by:

1. @redshark1802

@mkosowsk please take a look at the submitted work:

• PR by @redshark1802

---

• Learn more on the Gitcoin Issue Details page
• Want to chip in? Add your own contribution here.
• Questions? Checkout Gitcoin Help or the Gitcoin Slack
• $54,694.37 more funded OSS Work available on the Gitcoin Issue Explorer

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

The funding of 70.0 DAI (70.0 USD @ $1.0/DAI) attached to this issue has been approved & issued to @redshark1802.

• Learn more on the Gitcoin Issue Details page
• Questions? Checkout Gitcoin Help or the Gitcoin Slack
• $54,624.37 more funded OSS Work available on the Gitcoin Issue Explorer

[yohanelly95]

Is there a resource with all the possible slither args? I am trying to slither with foundry so it would be helpful