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.

Sign up for GitHub

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

Fix parenthesized expressions in braceless if conditions #350

Merged
merged 3 commits into from
Sep 19, 2023
Merged

Fix parenthesized expressions in braceless if conditions #350

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f8d3848
Fix parenthesized expressions in braceless `if` conditions
susliko Sep 16, 2023
5c54434
Bump dotty codebase smoke test to 84
susliko Sep 18, 2023
c13de90
Merge branch 'master' into fix-parenthesized-expr-in-braceless-if
eed3si9n Sep 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions corpus/expressions.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,11 @@ class C:
else
()

if (a) || b(c) then return true
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!


if (a) && b.c then
()

--------------------------------------------------------------------------------

(compilation_unit
Expand Down Expand Up @@ -445,6 +450,27 @@ class C:
(indented_block
(unit)
(comment))
(indented_block
(unit)))
(if_expression
(infix_expression
(parenthesized_expression
(identifier))
(operator_identifier)
(call_expression
(identifier)
(arguments
(identifier))))
(return_expression
(boolean_literal)))
(if_expression
(infix_expression
(parenthesized_expression
(identifier))
(operator_identifier)
(field_expression
(identifier)
(identifier)))
(indented_block
(unit))))))))

Expand Down
37 changes: 21 additions & 16 deletions grammar.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ module.exports = grammar({
// 'for' operator_identifier ':' _annotated_type • ':' …
[$._type, $.compound_type],
[$.lambda_expression, $.modifiers],
// 'if' parenthesized_expression • '{' …
[$._if_condition, $._simple_expression],
// _postfix_expression_choice ':' '(' wildcard • ':' …
[$.binding, $._simple_type],
],
Expand Down Expand Up @@ -1106,22 +1108,25 @@ module.exports = grammar({
),

if_expression: $ =>
prec.right(
PREC.control,
seq(
optional($.inline_modifier),
"if",
field(
"condition",
choice(
$.parenthesized_expression,
seq($._indentable_expression, "then"),
),
),
field("consequence", $._indentable_expression),
optional(seq("else", field("alternative", $._indentable_expression))),
),
),
prec.right(PREC.control, seq(
optional($.inline_modifier),
"if",
field(
"condition",
$._if_condition,
),
field("consequence", $._indentable_expression),
optional(seq("else", field("alternative", $._indentable_expression))),
)),

// NOTE(susliko): _if_condition and its magic dynamic precedence were introduced as a fix to
// https://github.com/tree-sitter/tree-sitter-scala/issues/263 and
// https://github.com/tree-sitter/tree-sitter-scala/issues/342
// Neither do I understand why this works, nor have I found a better solution
_if_condition: $ => prec.dynamic(4, choice(
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm ashamed of this

$.parenthesized_expression,
seq($._indentable_expression, "then"),
)),

/*
* MatchClause ::= 'match' <<< CaseClauses >>>
Expand Down
2 changes: 1 addition & 1 deletion script/smoke_test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

SCALA_SCALA_LIBRARY_EXPECTED=100
SCALA_SCALA_COMPILER_EXPECTED=96
DOTTY_COMPILER_EXPECTED=83
DOTTY_COMPILER_EXPECTED=84
SYNTAX_COMPLEXITY_CEILING=1300

if [ ! -d "$SCALA_SCALA_DIR" ]; then
Expand Down
16 changes: 8 additions & 8 deletions test/highlight/basics.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,19 @@ object Hello {
trait Test {
// ^ keyword
// ^ type
def meth(i: Int)(implicit x: Boolean) = ???
def meth(i: Int)(implicit x: Boolean) = ???
Copy link
Collaborator Author

@susliko susliko Sep 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've made definitions in this trait to be of the same indentation because

trait Foo {
    def meth = ???
  def anonFun = ???
}

is parsed with an ERROR and this causes flacking of the highlighting test

We might want to open an issue on this, but I'm reluctant to it as scalac raises a warning for this code

-- Warning: foo.scala:3:2 ------------------------------------------------------
3 |  def anonFun = ???
  |  ^
  |  Line is indented too far to the left, or a `}` is missing

// ^keyword.function
// ^keyword
// ^type
// ^method
// ^parameter
// ^parameter

val anonFun: Int => Int = (a: Int) => a
// ^variable
// ^type
// ^operator
// ^type
// ^parameter
val anonFun: Int => Int = (a: Int) => a
// ^variable
// ^type
// ^operator
// ^type
// ^parameter
}

protected abstract class Bla(test: String)
Expand Down