Rename Expression variants

[jfmengels]

I think the names for the different expression variants can be made clearer, to make it easier for users to grasp.

FYI, at the moment, people who try elm-review tell me that the hardest part is working with elm-syntax (which does make sense, since that is a big part of the elm-review API ).

Application variant

Expression has a Application variant.

To this day, I find that name confusing and un-intuitive. I think that something like FunctionCall would be more understandable (to me at least), especially since with the changes proposed in https://github.com/stil4m/elm-syntax/issues/43, it would have one argument at the very least (that used to be true before, but this becomes more obvious to the user).

OperatorApplication variant

I think this one makes sense, but I feel like Operation would be more intuitive, although it could be not clear enough :thinking:

(By the way, maybe for a different issue, but does the InfixDirection argument bring any value here after post-processing?)

Operator variant

I have no clue what this represents in practice, I think this one should not exist.

Literals

There are several kinds of literals that are not consistent. I think each literal type could be suffixed with Literal:

• CharLiteral: This one is already good :+1:
• Floatable is a bit of an odd name I think. FloatLiteral is better IMO
• ListExpr -> ListLiteral
• For consistency, Integer -> IntLiteral, Hex -> HexLiteral
• Lastly: Literal -> StringLiteral. It is not clear that Literal pertains to strings, so I think it is nice to explicit that.

Other renames

I think the names for expression variants can also be re-thought to be more consistent. A lot of them are suffixed with Expr, some with Expression, others with Block and some are not suffixed, leading to an inconsistent experience.

I think we can remove Expr/Expression from most, while still making a lot of sense.

• IfBlock could be renamed to If (or at least IfExpr, since the Elm guide calls them if expressions)
• UnitExpr could be renamed to Unit. I don't think there is any confusion here
• TupledExpression -> Tuple. Although this could conflict with the proposal I made in #49 when people import everything with (..). So either we discourage doing so, or this could be named TupleExpr
• ParenthesizedExpression -> Parenthesized? Not entirely sure about this one, but I think it makes sense
• LetExpression -> Let (I personally like LetIn, but that's not what other people call those)
• CaseExpression -> Case (I personally like CaseOf, but that's not what other people call those)
• LambdaExpression -> Lambda
• RecordUpdateExpression -> RecordUpdate
• GLSLExpression -> GLSL?

Some context

FYI, I tend to import and use elm-syntax expressions like this:

import Elm.Syntax.Expression as Expression exposing (Expression)

foo = case Node.value node of
  Expression.RecordUpdateExpression -> ...

So that it becomes easier to know where a value comes from, which I feel can become overwhelming otherwise. When doing so, having names like Expression.RecordUpdateExpression becomes a mouthful that does not add any additional information.

I wouldn't be against suffixing some variants with Expr/Expression as they are now, but I am not sure that would bring a lot of value.

[sparksp]

OperatorApplication variant

If Application is renamed like FunctionCall then would this make sense as OperatorCall?

Operator variant

I have no clue what this represents in practice, I think this one should not exist.

I believe it is used to represent operators used standalone, like (-) and (++).

[jfmengels]

If Application is renamed like FunctionCall then would this make sense as OperatorCall?

OperatorApplication represents expression like 1 + 2, which does not look like a "call" to me. That is commonly known as an operation in mathematics, which is also the term used in the core library's documentation.

I think Operation is actually a good term now. My indecision on this has ended :smile:

I believe it is used to represent operators used standalone, like (-) and (++).

Those are covered by PrefixOperator.

[sparksp]

You're right! PrefixOperator is a (-) or (::). It seems that the Operator is the actual operator, instances are created in src/Elm/Parser/Declarations.elm:583. And it comes up a lot in the tests. Although OperatorApplication and PrefixOperator but use String not Operator in them!

[stil4m]

Operator is actually only used during parsing as far as I can see. It is an intermediate value.

The following syntax (foo bar baz + 1 * 3) is broken up into an List (foo,bar, baz,+, 1, ect.). Based on operator precedence a valid syntax tree is build. It would be better to eliminate Operator, but I'm not sure how yet.

Regarding the change to remove the Expression suffix from the Expression value constructors. I like it. I had the same idea in my head for Pattern.

The other renames you suggest I like.

I'm not sure on Operation though. Need to let that one sink in for a moment.

[jfmengels]

It would be better to eliminate Operator, but I'm not sure how yet.

I guess you pretty much have to have a different type for the intermediary AST, which should be either entirely hidden or opaque to the users of the library (unless having that serves some kind of purpose).

[jfmengels]

I just noticed that the elm-in-elm project also has an expression type.

https://package.elm-lang.org/packages/elm-in-elm/compiler/latest/Elm-AST-Canonical#Expr

@janiczek if you have any insights/opinions to share, please let us know :)

[Janiczek]

:wave: Hello! Probably better to link to the Frontend Expr :) https://github.com/elm-in-elm/compiler/blob/master/src/Elm/AST/Frontend.elm#L53

Anyways we're not feature complete yet so the comparisons probably won't be useful (eg. custom operators vs our hardcoded Plus, Cons etc. for now.

And looking at elm-syntax Expression type, it just makes me realize how much elm-in-elm's Frontend.Expr will have to change. Eg. we totally discard the specifics of how the user code looked, like having only one Int and forgetting if it was hex or not. To support elm-format-like, or even elm-review/elm-analyse-like scenarios, I believe we'll have to move in the direction of elm-syntax, not the other way round. Kudos to @stil4m :slightly_smiling_face:

---

Anyways I like the renamings you've mentioned in this issue.

@stil4m I might not understand the problem with Operator fully, but if you're searching for a way to get rid of the Operator or some other variants that aren't useful to the user / aren't used in the final parsed value, perhaps you can have two Expr types, one used for internal parsing and then converted into the other, simplified one (this one you'd expose and publish). Thus users would not have to think about the seemingly unused cases.