Regression in 1.7.0: Parser rejects valid class def syntax with whitespace after class name

[toddsundsted]

Bug Report

I didn't see this called out in the changelog or previous issues as an intentional change, so I'm raising it as a possible compiler regression.

The following code compiles in versions of the compiler up to 1.6.2:

module Foo end
class Bar include Foo end

It no longer compiles in 1.7.x. The following does compile, as expected:

module Foo end
class Bar
  include Foo
end

[Blacksmoke16]

Caused via #12622. Which was listed as a breaking change, but still might be a regression in this particular case.

Also I didn't realize you could have inlined declarations like that without using ;.

[asterite]

I think we should fix it. It works in Ruby just fine and I see no reason to disallow such syntax.

[straight-shoota]

Yeah, that breaking is not acceptable.

We should probably just revert #12622 and then figure out what we can salvage from it.

[Sija]

IMO such a breaking change is better than the syntax with even more exceptions.

[asterite]

The thing is that there were no exceptions before. And none was needed. class Foo include Bar end was fine. class Foo:bar end also kind of works: it's a class name followed by a symbol. It works in Ruby too. It doesn't look good, but it makes sense from a lexer/parser perspective. We wanted to fix that (the Foo:bar thing which looks strange) and with it we broke something else.

[straight-shoota]

@Sija If it breaks working code, it's never better. In some cases it can be unavoidable to fix serious bugs.

But in this case, it's completely avoidable.

That doesn't mean that the changes from #12622 are undesired. But they're not necessary. We can postpone them, tinker with it and decide what syntax should be accepted and what not. We were expecting this to only affect obscure edge cases that are never used. No big deal. But apparently it affects real code, and that we should prevent.

[toddsundsted]

thank you, all!

[straight-shoota]

Resolved by #12977