crystal icon indicating copy to clipboard operation
crystal copied to clipboard

Premature closure of a class can be missed because many class operations are valid at top-level

Open BrucePerens opened this issue 2 years ago • 3 comments

This is a common typo:

class Foo
  def a
  end
  end # <--- This closes the class prematurely.

  def [](a : Int32)
     1
  end

  def initialize
  end

  private def b
  end

  protected def c
  end

Generally the first complaint Crystal emits will be about an end statement that's meant to close the class, when it was expecting EOF. However, it's just passed a lot of stuff that shouldn't have meaning at top-level, and is more confusing to the user than not if it does.

BrucePerens avatar Aug 05 '22 20:08 BrucePerens

See https://github.com/crystal-lang/crystal/pull/10019

asterite avatar Aug 05 '22 20:08 asterite

However, it's just passed a lot of stuff that shouldn't have meaning at top-level

But defining methods at the top-level is fine. The above snippet is perfectly valid. That's what makes it hard to detect.

I'd strongly suggest using the formatter integrated into the editor. With the formatter, the above snippet looks like this to me:

class Foo
  def a
  end
end # <--- This closes the class prematurely.

def [](a : Int32)
  1
end

def initialize
end

private def b
end

protected def c
end

The formatter is not just a formatter: it helps detects errors like the above one.

asterite avatar Aug 05 '22 20:08 asterite

I could see there being an error (or warning at least) for the protected def c method, given that doesn't have any defined semantical meaning, unlike private. Also is there even a way to call def [](a) on the top level? I'm thinking no?

The other two methods are valid I'd say.

EDIT: Or keep it valid but have the formatter remove protected?

Blacksmoke16 avatar Aug 05 '22 20:08 Blacksmoke16