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

Rewrite automatic unit insertion #214

Closed
gabro opened this Issue Jun 12, 2017 · 5 comments

Comments

Projects
None yet
2 participants
@gabro
Collaborator

gabro commented Jun 12, 2017

// before
val x: Option[Unit] = Option()

// after
val x: Option[Unit] = Option(())

As discussed in #208, we should include this into NoAutotupling, and rename it to NoAdaptedArgs in order to match the scalac flag that detects both cases.

The implementation should be simple enough, since the compiler provides detailed messages for this case too.

@gabro gabro added the rule label Jun 12, 2017

@olafurpg

This comment has been minimized.

Show comment
Hide comment
@olafurpg

olafurpg Jun 12, 2017

Member

This would be a nice rewrite. Question is whether we should bundle this rewrite with NoAutoTupling or keep it separate. I think it might make more sense to keep it separate as NoUnitInsertion. We can compose the two rewrites

val NoAdaptedArgs = NoUnitInsertion.andThen(NoAutoTupling)
Member

olafurpg commented Jun 12, 2017

This would be a nice rewrite. Question is whether we should bundle this rewrite with NoAutoTupling or keep it separate. I think it might make more sense to keep it separate as NoUnitInsertion. We can compose the two rewrites

val NoAdaptedArgs = NoUnitInsertion.andThen(NoAutoTupling)
@olafurpg

This comment has been minimized.

Show comment
Hide comment
@olafurpg

olafurpg Jun 12, 2017

Member

For rewrites like this it might be useful to have some kind of PositionPatch that works on positions, that would be an even lower-level patch than token patches. What do you think?

Member

olafurpg commented Jun 12, 2017

For rewrites like this it might be useful to have some kind of PositionPatch that works on positions, that would be an even lower-level patch than token patches. What do you think?

@gabro

This comment has been minimized.

Show comment
Hide comment
@gabro

gabro Jun 12, 2017

Collaborator

We can compose the two rewrites

val NoAdaptedArgs = NoUnitInsertion.andThen(NoAutoTupling)

I like that! Composition FTW 👍

For rewrites like this it might be useful to have some kind of PositionPatch that works on positions

Yes, especially for rewrites working with compiler messages, some position-based operations would be extremely useful!

Collaborator

gabro commented Jun 12, 2017

We can compose the two rewrites

val NoAdaptedArgs = NoUnitInsertion.andThen(NoAutoTupling)

I like that! Composition FTW 👍

For rewrites like this it might be useful to have some kind of PositionPatch that works on positions

Yes, especially for rewrites working with compiler messages, some position-based operations would be extremely useful!

@gabro

This comment has been minimized.

Show comment
Hide comment
@gabro

gabro Jun 12, 2017

Collaborator

I also think this would be a good first contribution, taking NoAutotupling as reference implementation.

Collaborator

gabro commented Jun 12, 2017

I also think this would be a good first contribution, taking NoAutotupling as reference implementation.

@olafurpg

This comment has been minimized.

Show comment
Hide comment
@olafurpg

olafurpg Oct 6, 2017

Member

Fixed in #352

Member

olafurpg commented Oct 6, 2017

Fixed in #352

@olafurpg olafurpg closed this Oct 6, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment