Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Tracking] Ideas for Printer CLI & API improvements #759

Open
snd opened this issue Jan 6, 2021 · 6 comments
Open

[Tracking] Ideas for Printer CLI & API improvements #759

snd opened this issue Jan 6, 2021 · 6 comments

Comments

@snd
Copy link
Contributor

snd commented Jan 6, 2021

A place to collect ideas for improving the printer system.

CLI

  • add --filter {contract} to only output results for the given {contract}
    • reduces noise when just looking at a single contract
    • requires printer system to be aware which outputs belong to which contract
  • add --filter {contract}.{function} to only output results for the given {function} of {contract}
    • allows to quickly find out things about a specific function
    • requires printer system to be aware which outputs belong to which function
  • Consider adding classification to the printers (for example Function Explorer, Summary, SlithIR, ...), and adding these categories to printer table (with an order by classification, and then name)
  • add --printer-doc {printer-id} to output reference for given {printer-id}
  • change --markdown "" to --printers-wiki-markdown and --detectors-wiki-markdown
  • make --json output something that has more structure than a table

API

  • introduce way for printers to communicate that certain results belong to certain contracts and functions
    • enables --filter (see above)
    • enables better json output that has standardized grouping for contracts and functions and is easier to consume by other programs
    • enables automated inclusion of source mapping in --json output which is needed if slither vscode plugin wants to display printer results inline
  • Add the printer documentation to the python class (similar to what we have for the detectors). This will ensure we keep an up to date documentation for each printer and will let us update the wiki page entirely in one step (it's how we do it for detectors).
  • remove param filename from AbstractPrinter.output(...) since printers generally ignore it and it is also available as self.filename
@montyly
Copy link
Member

montyly commented Jan 7, 2021

This is great.

We can probably merge #761 here.

Instead of filter, maybe we can use exclude and include (or similar names, but that works in both directions). We can then use the same options for detectors and printers, so something like

  • --exclude-filenames / --include-filenames
  • --exclude-contracts / --include-contracts
  • --exclude-functions / --include-functions. Maybe with both support for f(..) and contract.f(..), so we work with top level functions, and can filter similar functions across contracts in case of inheritance

For --printer-doc, I am not sure as the description will be in markdown, so it's likely to be difficult to read from the terminal. An alternative is to show the wiki url after the printer result (it's what we do with detectors).

For --markdown, what is the goal? As far as I know, this flag is only used to update the README.md/wiki (mostly on each release). This is why this flag is hidden and not shown in the --help. If we have other usages in mind, we should revisit if it should stay hidden. Note that te second argument ("") is not used to differentiate detectors/printers, but public/private content.

For --json, note that the printers have different json output (ex: they can return a file, a custom dictionary, ...). My understanding is that you refer to the printers that output a pretty table, is that correct?

@snd
Copy link
Contributor Author

snd commented Jan 7, 2021

I really like the idea of using the same options for detectors and printers and using include and exclude!

For --markdown, what is the goal? As far as I know, this flag is only used to update the README.md/wiki (mostly on each release). This is why this flag is hidden and not shown in the --help. If we have other usages in mind, we should revisit if it should stay hidden. Note that te second argument ("") is not used to differentiate detectors/printers, but public/private content.

I thought --printers-wiki-markdown and --detectors-wiki-markdown would be self explanatory. --markdown "" is not in my opinion. They could still remain hidden.

For --json, note that the printers have different json output (ex: they can return a file, a custom dictionary, ...). My understanding is that you refer to the printers that output a pretty table, is that correct?

Yes. I'm thinking in the direction of keeping the flexibility of printers writing files and returning custom json but giving them a simple way to output data that is grouped into contracts and functions from which tables, json in a somewhat standardized format and more can be generated. I hope that makes sense.

@snd
Copy link
Contributor Author

snd commented Jan 7, 2021

With exclude/include if I'm just interested in a single contract (or function) A would I need to --exclude-contracts --include-contracts A? that seems very verbose.

Alternatively --include... could exclude all by default unless there's an --exclude... before it.

Wondering if there's a problem with that.

@montyly
Copy link
Member

montyly commented Jan 7, 2021

Yes, if you use --include-.. X, then only X will be shown (not need to use --exclude), in the same way we have --detect DETECTOR_NAME and --exclude DETECTOR_NAME (either a whitelist, or a blacklist).

But here we might want to allow the combinations (ex: --include-functions add --exclude-contracts SafeMath would return all the add functions that are not in SafeMath).

@devtooligan
Copy link
Contributor

Some additional improvements to consider:

  • in addition to filtering by contracts it would be nice to have a flag to select only Fully Derived contracts
  • it would also be nice to have a mechanism to be able to pass specific args to specific printers that would be dependent upon the printer.
  • overhauling the output is another huge area for improvement. could have each printer print to a specific output file by default and allow for that to be customized

@0xalpharush
Copy link
Contributor

0xalpharush commented Jun 27, 2023

@0xalpharush 0xalpharush removed this from the 0.9.4 milestone Jun 27, 2023
@0xalpharush 0xalpharush changed the title [WIP] Ideas for Printer CLI & API improvements [Tracking] Ideas for Printer CLI & API improvements Jun 27, 2023
@0xalpharush 0xalpharush added the ux label Sep 8, 2023
@coderabbitai coderabbitai bot mentioned this issue Apr 26, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants