inaka / elvis_core

The core of an Erlang linter
Other
61 stars 56 forks source link

New `elvis_style` rule: `no_init_lists` #367

Closed bormilan closed 3 weeks ago

bormilan commented 1 month ago

Description

A brief description of your changes.

Closes #329;.


EDIT: I made a checklist based on @elbrujohalcon 's comment to to make the progress traceable.

bormilan commented 1 month ago

Do we want to check that the module implements a gen_* behavior, or should we check every module like now? Oh, and I want to check if this rule can work on .beam files.

paulo-ferraz-oliveira commented 1 month ago

I tweaked the PR title since this'll go directly into the generated release Changelog.

bormilan commented 1 month ago

Thanks for the comments, I will solve all of them if I have some time. ❤️

bormilan commented 1 month ago

I still have some work to do, but it keeps getting better.

elbrujohalcon commented 1 month ago

@bormilan do you mind merging main into your branch, please?

bormilan commented 1 month ago

One more test (or maybe two, if you want a fail and a pass), @bormilan:

  • A module that implements gen_server AND another behaviour…
-module(fail…).

-behaviour(gen_server).
-behaviour(another_behaviour).

-export([init/1]).

init([]) -> {error, "should not be a list"}.

Yes! I like making more and more tests to feel that my code is 100% good.

elbrujohalcon commented 1 month ago

Yes! I like making more and more tests to feel that my code is 100% good.

HA! I just saw that you were matching against a single [BehaviourNode] in your code… and I wondered "what would happen if we have multiple ones?"… so… we have to test that, right?

bormilan commented 1 month ago

Yes! I like making more and more tests to feel that my code is 100% good.

HA! I just saw that you were matching against a single [BehaviourNode] in your code… and I wondered "what would happen if we have multiple ones?"… so… we have to test that, right?

You're right. I'm working on the new logic right now, so it's the perfect time to add new tests and consider all the possible barriers.

bormilan commented 1 month ago

The only test that is not working now is the one with the init([]) because its attribute type is nil. I don't know how I will solve this, it may need some extra magic. Or is it a bug in katana_code?

elbrujohalcon commented 1 month ago

The only test that is not working now is the one with the init([]) because its attribute type is nil. I don't know how I will solve this, it may need some extra magic. Or is it a bug in katana_code?

Isn't it as simple as lists:member(ktn_code:type(Attr), [cons, nil])?

I think nil is precisely [], which is fine.

bormilan commented 1 month ago

The only test that is not working now is the one with the init([]) because its attribute type is nil. I don't know how I will solve this, it may need some extra magic. Or is it a bug in katana_code?

Isn't it as simple as lists:member(ktn_code:type(Attr), [cons, nil])?

I think nil is precisely [], which is fine.

Oh! Okay, that's nice, I started to dig into the code of katana, thanks for the clarification.