prettier-solidity / prettier-plugin-solidity

A Prettier plugin for automatically formatting your Solidity code.
https://t.me/+kgTgkFgIwJkwMjcx
MIT License
727 stars 73 forks source link

printer #2

Closed mattiaerre closed 6 years ago

mattiaerre commented 6 years ago

Description

what is the best strategy to implement the print function? Reading the documentation is not clear to me what it should happen when there is an AST returned from a parser.

should I read this: https://homepages.inf.ed.ac.uk/wadler/papers/prettier/prettier.pdf ?

should I watch this: https://www.youtube.com/watch?v=0Q4kUNx85_4&feature=youtu.be ?

any other recommendations?

// cc @lipis @duailibe @azz @czosel @kachkaev @warrenseine @ikatyang @j-f1

azz commented 6 years ago

I'd say start by reading a simple printer, like this one, and read the commands docs for info on what each doc builder does.

mattiaerre commented 6 years ago

thanks @azz the printer you mentioned is indeed simple; much appreciated 😅

mattiaerre commented 6 years ago

another question for you guys: in order to build the plugin do I have to implement the following patter?

text -> AST -> IR -> text

I am evaluating to take advantage of solium for my genericPrint function but it has got it's own internal parser; what would you recommend?

// cc @lipis @duailibe @azz @czosel @kachkaev @warrenseine @ikatyang @j-f1 @duaraghav8

kachkaev commented 6 years ago

prettier-plugin-elm delegates both parsing and printing to run-elm, so you might find it useful to see how that’s done there.

j-f1 commented 6 years ago

@mattiaerre

I am evaluating to take advantage of solium for my genericPrint function but it has got it's own internal parser; what would you recommend?

I personally feel like this would go against Prettier’s goals. What Prettier tries to do is to reprint the program’s source from the AST, ideally without looking at the original code other than to parse it to get the AST. It looks like solium is a linter, which has a different goal. It is designed to check the program’s source for problematic patterns and fix them by modifying parts of the source. Although useful, it is a different goal than Prettier.

I’d recommend creating your own printer.

To do so, you would typically put a big switch in your genericPrint function and add a case for each node type. The case should return the IR, composed of the commands @azz linked above, and use path.call(print, "key") to recurse (in this case, it passes a path to print() with the value of path.getValue().key as the new path.getValue(). After all of the cases, you can put a default. In the beginning, the default case could contain code to print the passed node exactly as written (by reading its source code) to allow you to gradually add support for the entire Solidity language while still being able to test real code.

mattiaerre commented 6 years ago

thanks @j-f1 your comment is really helpful for me to better understand the prittier's philosophy. I will be doing what you are suggesting and incrementally write the print function based on the AST produced by the parser; I like very much this approach. thanks again, I'll keep you posted. out of curiosity: in this case, the approach suggested by @kachkaev looks like more a shortcut as there is no "real" AST here; for sure I'm missing something. what do you think?

mattiaerre commented 6 years ago

hi @j-f1 I was trying to implement your idea of incrementally format nodes based on known types; the point that I'm struggling w/ is really how to handle the default case as there is no clear understanding for me how to "print the passed node exactly as written (by reading its source code)". do you have an example to share? anything you think I should be looking at?

// cc @federicobond

j-f1 commented 6 years ago

I don’t have a code sample for the default case. However, the algorithm should be pretty simple. You have to find the start and end index of the node in the file (this should be an attribute of the node), then return options.originalText.slice(start, end).

federicobond commented 6 years ago

Hi @mattiaerre, I am very interested in seeing this project done. I actually attempted to do some similar work a while ago and left it for other, more pressing, issues.

@j-f1 suggestions for the default case is right. You can access the range when configuring the parser correctly and then it's a matter of slicing the range you want.

mattiaerre commented 6 years ago

thanks so much for your input and ideas @j-f1 and @federicobond I've just created a simple PR to start the print workflow #4 what do you think? I'm adding you as collaborators to this repo

mattiaerre commented 6 years ago

closing this issue because of #4