eclipse-archived / ceylon.formatter

A formatter for the Ceylon programming language, written in Ceylon.
Apache License 2.0
14 stars 11 forks source link

Grouped types #23

Closed lucaswerkmeister closed 10 years ago

lucaswerkmeister commented 10 years ago

In the AST, there is no difference between the following declarations:

String s1;
<String> s2;

Damn.

I can try to recognize where grouping is necessary (like in {<Key->Value>*}), and only group the type then, but that leaves the problems that I might miss corner cases, and (much worse) that if the user overuses grouping (perhaps for readability since type operator’s precedence is unclear), we output code that differs from the input code in more than just whitespace.

The other solution would be to add some notion of “optional” tokens to the FormattingWriter: “only write out this token if you find a corresponding one in the TokenStream”. But that would be operating on the token level, and in some cases an optional token and an adjacent required token might collide. And the FormattingWriter is already complicated enough as it is :-/

@gavinking would it be possible to add a GroupedType extends StaticType to the AST?

gavinking commented 10 years ago

I can certainly add a special Node type, but all the info is there, isn't it, as the token and lastToken of the Node?

Sent from my iPhone

On 14 Feb 2014, at 6:24 pm, Lucas Werkmeister notifications@github.com wrote:

In the AST, there is no difference between the following declarations:

String s1;

s2; Damn. I can try to recognize where grouping is necessary (like in {Value>*}), and only group the type then, but that leaves the problems that I might miss corner cases, and (much worse) that if the user overuses grouping (perhaps for readability since type operator’s precedence is unclear), we output code that differs from the input code in more than just whitespace. The other solution would be to add some notion of “optional” tokens to the FormattingWriter: “only write out this token if you find a corresponding one in the TokenStream”. But that would be operating on the token level, and in some cases an optional token and an adjacent required token might collide. And the FormattingWriter is already complicated enough as it is :-/ @gavinking would it be possible to add a GroupedType extends StaticType to the AST? — Reply to this email directly or view it on GitHub.
lucaswerkmeister commented 10 years ago

No, currently token and endToken are both null. But I suppose that works as well, and it’s probably a lot easier.

lucaswerkmeister commented 10 years ago

I must be missing something obvious... I just tried to make the grammar change, but I can’t find a way to add the token < to the type. There’s setEndToken(), but not setToken(), because the token is a constructor argument – but I’m not constructing the type in groupedType, so I can’t give a constructor argument! Relevant line of the grammar Solution? Create a new StaticType and copy the fields?

lucaswerkmeister commented 10 years ago

Argh, of course I can’t create an instance of the abstract class StaticType.

Bigger problem: some types, like <{String*}>, already have tokens ({ and }), so we can’t store the < and > there. I don’t think there’s a grammar-wise solution apart from a new GroupedType :-(

(Again, you don’t need to add this. It would be convenient, but I can work around the restriction.)

lucaswerkmeister commented 10 years ago

On the other hand, a new GroupedType class breaks all kinds of instanceof UnionType checks... it’s probably better if I add optional tokens to the TokenWriter. (Although I’ll still need to figure out where grouping is required, for the case where there’s no TokenStream.)

lucaswerkmeister commented 10 years ago

Okay, optional tokens were ridiculously easy to implement, I shouldn’t have been afraid of them. Sorry to bother you with all that @gavinking!

lucaswerkmeister commented 10 years ago

Well, looks like it’s not quite as easy.

Range<Integer> r = 0..1;

The <> around Integer are required (type argument list), but you might additionally group the Integer again. So I first visit the type argument list, write out a non-optional <. Then I visit Integer. Integer says it has optional <> around it. The FormattingWriter doesn’t find a <, but it does find and consume >. But now the > for the type argument list is gone – exception.

I need to be able to determine if the optional < was found, and only then write the >. (Also, this needs to happen in a loop: <<<Integer>>> is a perfectly legal type.)

lucaswerkmeister commented 10 years ago

I need to be able to determine if the optional < was found

Well, duh, no problem at all! writeToken returns a FormattingContext if the token was written, and null otherwise. No need for any special magic here!

(By the way, Gavin, feel free to unsubscribe from the issue if you haven’t already. This no longer concerns you.)

gavinking commented 10 years ago

:)

lucaswerkmeister commented 10 years ago

Dangit, of course it’s not that easy :-(

<String>[] sx;
<String[]> sy;

I suppose the only solution, other than reimplementing the whole backtracking parser shenanigans myself, is to pretend that the <> of type arguments are optional as well, and to write out optional <s and >s as long as the FormattingWriter finds them, without any connection between corresponding brackets.