rrrene / credo

A static code analysis tool for the Elixir language with a focus on code consistency and teaching.
http://credo-ci.org/
MIT License
4.93k stars 417 forks source link

Custom check shipped with lib not loading #1068

Closed epinault closed 1 year ago

epinault commented 1 year ago

Environment

What were you trying to do?

I creating a credo plugin (that is really close to the example), and trying to ship also custom check as part of the plug into the same package Then I am trying to use the plugin and that custom check in an application. I was hoping to define the new check as part of the plugin to load so I don t have to define anything in the application

So say I have MyApp as application, and my Plugin is a separate package called MyCredoPlugin MyCredoPlugin .credo.exs file has this

%{
  #
  # You can have as many configs as you like in the `configs:` field.
  configs: [
    %{
      name: "default",
      requires: ["./lib/credo_podium/check/**/*.ex"],
      checks: [
        #without this line, it s does not seem to be running from the plugin config
        {Credo.Check.Refactor.ABCSize, []},
        ## this line does not seem to still enable the check in this plugin when running credo
        {CredoPodium.Check.Security.LogWithInspect, []}
      ]
    }
  ]
}

Expected outcome

I would expect the plugin to be enabled and running by default the new check. and see bunch of output that I know MyApp will trigger

Actual outcome

it s not running the check though it is loading the plugin correctly.

In order the enable that custom check. I had to add it to my .credo.exs inside the MyApp folder as such

%{
  #
  # You can have as many configs as you like in the `configs:` field.
  configs: [
    %{
      name: "default",
      plugins: [{MyCredoPlugin, []}],
      checks: [
        #without this line, it s does not seem to be running from the plugin config
        {CredoPodium.Check.Security.LogWithInspect, []}
      ]
    }
  ]
}

By the way I will likely send an MR for this check if you are interested in it.

rrrene commented 1 year ago

Here is a sample project which loads a demo plugin which brings its own check.

I think this demonstrates what you are trying to do, right?

If you are still having problems, can you create a minimal reproducible example?

epinault commented 1 year ago

yea this assumes that it s all in the same repo that example. I ll see if i can build a minimal repro next week

rrrene commented 1 year ago

yea this assumes that it s all in the same repo that example.

So, you have (a) a plugin in one repo, (b) a check in another repo and (c) a project where this should come together?

epinault commented 1 year ago

I have a lib that define the plugin and includes custom checks then I have an app that use that lib to configure credo and run the check defined by that lib

rrrene commented 1 year ago

And how is this different from the example linked above?

epinault commented 1 year ago

so I guess the setup is similar but I don t want the sample project to load the custom check. I put the require and the custom check in the demo plugin. So I was hoping not to have to have every application to have to enable the custom check in their credo file.

so I would say try to move the require and the custom check enabling in the plugin instead and that should about what I have

rrrene commented 1 year ago

So I was hoping not to have to have every application to have to enable the custom check in their credo file.

And you don't have to (again: see the linked example above).

I would very much like to help you (again: can you put something online that reproduces this error, so I can look at it?).

epinault commented 1 year ago

there you go!

clone both https://github.com/epinault/credo_sample_project and https://github.com/epinault/credo_demo_plugin both in the same dir

if you run mix credo,, you ll noticed I added a debug that show it s never running the Better check

rrrene commented 1 year ago

If I run mix credo suggest I get this:

image

Do you get another result?

epinault commented 1 year ago
❯ elixir -v
Erlang/OTP 25 [erts-13.2.2.1] [source] [64-bit] [smp:10:10] [ds:10:10:10] [async-threads:1] [jit]

Elixir 1.14.5 (compiled with Erlang/OTP 24)

❯ mix credo --help
Credo Version 1.7.0-ref.master.7179615+uncommittedchanges
Usage: $ mix credo <command> [options]

credo_sample_project on  master is 📦 v0.1.0 via 💧 on ☁️  (us-west-2) took 2s
❯ mix credo
By the power of !

only difference I see is you use suggest? I have it all over my CI or running locally as this

mix credo --strict

and now i see this error

❯ mix credo --strict ** (credo) Unknown switch for demo command: --strict

yet the lock file says

  "credo": {:hex, :credo, "1.7.0", "6119bee47272e85995598ee04f2ebbed3e947678dee048d10b5feca139435f75", [:mix], [{:bunt, "~> 0.2.1", [hex: :bunt, repo: "hexpm", optional: false]}, {:file_system, "~> 0.2.8", [hex: :file_system, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "6839fcf63d1f0d1c0f450abc8564a57c43d644077ab96f2934563e68b8a769d7"},
rrrene commented 1 year ago

The demo plugin teaches you to

  1. add a config (for a check provided by the plugin)
  2. register a new command
  3. register a CLI switch to configure that command and
  4. set that command as the "default command"

You seem to have used that demo plugin, wanting to use the first point and have not stripped out the code responsible for the last three points.

Therefore, when you type mix credo, it does not do what you think.

Did you never think "Why is it printing 'By the power of !'??" ...

epinault commented 1 year ago

I did not realize this plugin was also doing another command. I ll modify once more the code to reflect more my project . let me fully get a commit for those repo that reflect and remove that CLI command

epinault commented 1 year ago

meanwhile would you mind looking at #1069 since really that would solving my problem if that was part of credo already anyway

epinault commented 1 year ago

ok so I can see it working . I am not sure why it was not working on mh other project but once I clobber my plugin package then it started to behave again correctly. Though both were using the exact same plugin and using path: to point to it.. I wonder if there is an issue around hex and some caching using path at time.

I literally had the same config between those project another one. and only thing that made it work was doing a

mix deps.clean my_plugin