upenn-cis1xx / camelot

A fully-modular OCaml style linter
https://opam.ocaml.org/packages/camelot/
Apache License 2.0
44 stars 5 forks source link

Issue warnings for illegal library code use #61

Open Zdancewic opened 3 years ago

Zdancewic commented 3 years ago

Camelot could report warnings for use of library functions such as List.length etc.

We'd have to check for explicit uses of List.* and for open List. (or maybe just any occurrence of the List identifier.

I'm not sure how general this functionality should be -- we wouldn't want to enumerate all of the standard library modules as "banned", even though in practice that is what we mean most of the time.

Vighnesh-V commented 3 years ago

I've spent some time peeking through this:

There's a nice ppx_tool called dumpast, which prints the AST for a given expression:

Here's what ocamlfind ppx_tools/dumpast -e "List.map" prints:

{pexp_desc = Pexp_ident {txt = Ldot (Lident "List", "map")};
 pexp_loc_stack = []}

So it seems like the library information is still encoded which means we can figure out which library they're using by checking for the Ldot constructor.

I'm also pretty sure we can do checks for things like let open List in and open List, but something that might be easier is to just have a list of flagged functions like '@' and 'map' etc. That way we can use the arthur files to sandbox those checks to the functions we don't want them using this stuff on.

Since camelot is meant to be general though, I might flag some of these rules as 120 specific either in comments or code somewhere, since ordinarily you would want to use list library functions over rolling your own.

TLDR; we can implement this - I'll take a look at how to do it this week.

KeenWill commented 3 years ago

I think we can still have it be general if you specify in the Arthur files which modules / functions you want to check for. It wouldn't be very helpful for other projects that don't care about using library functions, but they could at least disable the functionality.