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

Code to report deprecation warnings is in codegen instead of semantic #8259

Open
asterite opened this issue Oct 2, 2019 · 5 comments
Open

Comments

@asterite
Copy link
Member

asterite commented Oct 2, 2019

The code to report deprecation warnings is in codegen/warnings.cr, which is super strange because warnings is a semantic thing. For example if you compile with --no-codegen you don't get the warnings.

I think this should be refactored. The best place to put this might be in CleanUpVisitor.

I might try to improve this, I just don't want to forget about it.

@bcardiff
Copy link
Member

bcardiff commented Oct 2, 2019

It can be moved around sure, but I would prefer to keep the co-location by feature.

@asterite
Copy link
Member Author

asterite commented Oct 2, 2019

What's "co-location by feature"?

@bcardiff
Copy link
Member

bcardiff commented Oct 4, 2019

That the code that makes a features is located near / in the same file. Instead of laying out the code by namespace. The last refactoring on unions.cr is an example of that.

@RX14
Copy link
Contributor

RX14 commented Oct 6, 2019

I've been working on and off on refactoring the warnings code towards adding a crystal tool warnings tool and then minimizing the verbostity of warnings output on crystal build. Part of this is abstracting printing frames from the compile error exception.

Part of this I moved the detection to the semantic phase, not codegen, and since the code is only useful for deprecation warnings, I might rename it all to deprecations instead.

@HertzDevil
Copy link
Contributor

There could definitely be other kinds of warnings, this one from 0.35.1 for example, which were also absent if --no-codegen was given:

abstract class Foo
  abstract def foo : Int32
end

class Bar < Foo
  def foo
  end
end
 6 | def foo
         ^--
Warning: this method overrides Foo#foo() which has an explicit return type of Int32.

Please add an explicit return type (Int32 or a subtype of it) to this method as well.

The above warning will become an error in a future Crystal version.

A total of 1 warnings were found.

Same goes for anything that we might want to disallow in 2.0.

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

No branches or pull requests

4 participants