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

linting old coding style #305

Open
chenkai036 opened this issue Nov 28, 2019 · 4 comments
Open

linting old coding style #305

chenkai036 opened this issue Nov 28, 2019 · 4 comments
Assignees

Comments

@chenkai036
Copy link
Contributor

chenkai036 commented Nov 28, 2019

hobbes language syntax was refined since commit cf789e0d1: it also introduces a different evaluation result for expressions like if true then 1 else 2 + 3, because the earlier versions interpret the expression as (if true then 1 else 2) + 3 and the later ones regard it as if true then 1 else (2+3).

I've added a small lint tool to capture such legacy coding style during loading hobbes modules or evaluation expressions. For some reason I cannot make OSX build pass on travis-ci. Perhaps it would be interested to you and I'd appreciate your comments, @kthielen @smunix

$ cat bad.hob
result = if true then 1 else 2 + 3

$ hi --lint bad.hob
lint: bad.hob:1,10-34: |INFIX_AFTER_ELSE|, use brackets in else-block to avoid ambiguity
loaded module 'bad.hob'
> if true then 1 else 2 + 3
lint: stdin:1,1-25: |INFIX_AFTER_ELSE|, use brackets in else-block to avoid ambiguity
1
> if true then 1 else (2 + 3)
1
> if true then 1>2 else 2 in [3]
false
@chenkai036 chenkai036 self-assigned this Nov 28, 2019
@kthielen
Copy link
Contributor

That's an interesting idea. I've just looked at the code a little bit, but it could be useful if the user also has access to hook into this mechanism, for example to define linting rules related to usage of his custom values and types.

On that particular syntax change, I can see how that would have been frustrating. It was requested by ... a person who was running an important project ... and honestly it's how the syntax should have been at the outset.

There's another option for this specific syntax issue, which is that hobbes::cc lets you install custom parsers (any function that takes a string to an ExprPtr) and so the old syntax could be kept in circumstances where somebody has legacy hobbes code that they don't want to modify.

There's an LALR(1) parser generator in hobbes (in hobbes/parse/parser.H), which can be used in the syntax to define local parsers. I've planned to have an option to use this to extend the hobbes syntax within hobbes (so that scripts can define new syntax). This would give us an option to do that "syntax override" method just with plain scripts rather than explicitly substituting function pointers.

Anyway, a tool like this can be useful for people in some situations, it's a good idea.

@kthielen
Copy link
Contributor

Also happy Thanksgiving Kai! It's good to hear from you. :)

@chenkai036
Copy link
Contributor Author

@kthielen happy Thanksgiving too! These options you suggested did cross my mind: they're more general solutions which require a few building blocks that represent the entire hobbes parser rules (maybe more). Using a custom parser to rewrite the legacy syntax to the new one looks nice (it still requires a full coverage of hobbes parser rules, if I understand you correctly). But I think we're probably not yet at the stage where the legacy code is too costly to change. I'm a little unsure whether it would cause potential confusions when the legacy code is mixed with the new syntax in the future.

Therefore I chose something simpler. The caveat is that a few lexical tokens that are discarded in the current flex/bison parsing context need a reconsideration, e.g. surrounding parenthesis for nested expressions. They're probably not a good candidate as ExprPtr subclasses and it makes little sense to switchOf over them; that's why I extend LexicalAnnotation a little bit.

BTW, I was also thinking whether a standalone lint process/daemon would make more sense, making it possible for editor plugins. Anyway it's not happening at the moment.

@kthielen
Copy link
Contributor

Yeah, maybe also your linter idea is orthogonal to the parsing issues since you could use it for "semantic patterns" too.

The parser/syntax part of hobbes could really use some TLC. Unfortunately it's always been a lower priority (at least for me) compared to the other features we've accumulated over the years. Some of the ideas there (like user-defined syntax) go against the grain of standard expectations of a fixed grammar (makes things like editor plugins tricky too).

OTOH, if we always do things the same way then we'll always have the same limitations. :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants