Crystal code check tool

I would like to propose to add another tool in the compiler. Something that will check some rules in the code base that aim for helping the developer for future versions (of the language, std lib or shards)

The first thing would be to check if an obsolete method is called. This will not only help users of the std lib but also simplifying the work on shards to advertise future breaking changes. Authors will find value on implementing some deprecation policy.

Another use is that some breaking changes in the language itself are hard to detect at first eyesight.

Having this tool detect beforehand it will also allow us to see a list of issues rather than stop at the first compilation error once we are migrating.

I’m ok with breaking changes but I would prefer to have a migration path as smooth as possible.

Specific cases were I would found this valuable:

  • Rename methods like #5346, #6533, #6662 there is no real reason to force a breaking change when introducing the new names.

  • Reference relevant information regarding a change like in #6598

  • Avoid having to manually use macros to simulate a warning that would be annoying in every compilation. Like it is proposed here.

  • When primitives changes like a / b will change from Int to Float. That could be a silent change, but we already have // to start changing the code, but we are not amused by the work of checking when two integers are been divided in the current code base.

Implementing these checks is basically the same effort as building the code but not generating the output. It could be viewed as adding warning support in the compiler but with an opt-in flag or tool that will show them.

One alternative would be to contribute with ameba but currently, AFAIK it runs at the syntax level and all the examples above require almost a compilation of the process which means that ameba would need to build the compiler to run those rules.

8 Likes

Awesome! A nice tool to include new features for crystal extensions on current Code Editors! :tada: :+1:

Quick note/crazy idea:

that ameba would need to build the compiler to run those rules.

I’d like to see compiler plugins, with a documented interface to access the types/ast/etc… So we can build tools that uses the compiler without having to ship with a specific version of the compiler

3 Likes

I like this proposal. Are you aware of any similar tools for other language? Doesn’t hurt to include prior work in the design.

Adding this to ameba wouldn’t be a great idea. If ameba wants to support this, that’s fine, but it should first and foremost be a compiler tool. Ameba is about code style.

I would like to start by:

  • Add a @[Deprecated("...")] annotation with some description
  • Add a --check option to the build command so a warning will be printed per invocation of deprecation method

Currently the docs tool hightlights DEPRECATED: .... notes. It would be good to tag deprecated methods, but it’s not mandatory as a first implementation.

As I mention before, maybe some breaking changes detection need to be written within then compiler, like if we move out from with ... yield. But for a lot of API changes the annotation and the compiler option should be good enough I think.

Similar to the format tool, maybe is good to allow checking for the code only within specific location, so if there are warnings emitted by dependencies those wont bother the app author too much.

I’d suggest to use the @[Deprecated] annotation to show a message in the API docs.

And maybe it should be a separate command from build, with a check-only mode and maybe (optional, later) even automatically apply changes.

Cons of doing a separate command:

  • We need to parse all the build flags (spec and run have had already some inconsistencies in this aspect already)
  • The hard part is the semantic analysis (without the codegen)
  • If someone cares about the check it can have the compiler with warnings in a single run
  • It’s not clear how to run the check against together with crystal spec
1 Like

Progress at https://github.com/crystal-lang/crystal/pull/7596