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

Improve opaque type with no RHS error message #15285

Merged
merged 2 commits into from
May 25, 2022

Conversation

nicolasstucki
Copy link
Contributor

@nicolasstucki nicolasstucki commented May 24, 2022

Fixes #15266

Done in the spree with @griggt and @mtk

@griggt
Copy link
Contributor

griggt commented May 24, 2022

It turns out this isn't the first time we've had issues with missing RHS of an opaque type, we have tests/neg/i6055.scala:

opaque type i0           // error: opaque type must be an alias type
opaque type i2 <: Int    // error: opaque type must be an alias type

opaque type i1[_]        // error: opaque type must be an alias type
opaque type x[_] <: Int  // error: opaque type must be an alias type

At one point the error message was as shown above, but this test has no check file, so the loss in clarity may not have been noticed at the time it was introduced.

More relevant for this PR, the new output for the above test is

-- Error: tests/neg/i6055.scala:1:12 -------------------------------------------
1 |opaque type i0           // error: opaque type must be an alias type
  |^^^^^^^^^^^^^^
  |opaque type must have a right-hand side
-- Error: tests/neg/i6055.scala:2:12 -------------------------------------------
2 |opaque type i2 <: Int    // error: opaque type must be an alias type
  |^^^^^^^^^^^^^^^^^^^^^
  |opaque type must have a right-hand side
-- [E156] Syntax Error: tests/neg/i6055.scala:4:12 -----------------------------
4 |opaque type i1[_]        // error: opaque type must be an alias type
  |^^^^^^^^^^^^^^^^^
  |Modifier opaque is not allowed for this definition
-- [E156] Syntax Error: tests/neg/i6055.scala:5:12 -----------------------------
5 |opaque type x[_] <: Int  // error: opaque type must be an alias type
  |^^^^^^^^^^^^^^^^^^^^^^^
  |Modifier opaque is not allowed for this definition

We should enhance the checking to recurse on LambdaTypeTree nodes so we detect the missing RHS on higher-kinded opaque types.

if !tree.mods.is(Opaque) then tree
else tree match
case TypeDef(_, bounds: TypeBoundsTree) if bounds.alias.isEmpty =>
report.error(i"opaque type must have a right-hand side", tree.srcPos)
Copy link
Contributor

@griggt griggt May 24, 2022

Choose a reason for hiding this comment

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

Suggested change
report.error(i"opaque type must have a right-hand side", tree.srcPos)
report.error(i"opaque type must have a right-hand side", tree.srcPos.endPos)

...if we want the caret to be placed where the missing RHS should go?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Both approaches seem reasonable to me. I do not have a preference on either option.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Both approaches seem reasonable to me. I do not have a preference on either option.

@griggt
Copy link
Contributor

griggt commented May 24, 2022

We should enhance the checking to recurse on LambdaTypeTree nodes so we detect the missing RHS on higher-kinded opaque types.

I pushed a commit to do this.

@nicolasstucki nicolasstucki requested review from smarter May 25, 2022 06:59
@nicolasstucki nicolasstucki marked this pull request as ready for review May 25, 2022 07:00
@smarter smarter merged commit e9de73c into scala:main May 25, 2022
@smarter smarter deleted the fix-15266 branch May 25, 2022 09:21
@Kordyjan Kordyjan added this to the 3.2.0 milestone Aug 1, 2023
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

Successfully merging this pull request may close these issues.

Misleading error message for opaque type
4 participants