hrzndhrn / recode

A linter with autocorrection and a refactoring tool.
MIT License
295 stars 15 forks source link

Remove parens #90

Closed sodapopcan closed 2 months ago

sodapopcan commented 3 months ago

Here's my first draft of removing parens.

I should be functionally complete though I still have several things to do.

Most importantly, I'm looking for some guidance on testing and general file to use to get the config opts. I'm using Mix.Tasks.Format.formatter_opts_for_file("mix.exs") in the implementation and have added a non-exported locals_without_parens: [assert: 1] to this project's .formatter.exs just for testing purposes. I don't love this, but since I have no clean way to do DI I need to either do it this way, use a mock (which means adding a library), or do some env-checking. Perhaps there is another way I'm not thinking of. What do you think?

Other than that I need to:

And once again I'd love some guidance on what to call this corrector.

Thanks!

NickNeck commented 3 months ago

Elixir has a bunch of defaults for locals without parens. We should also add these.

  @locals_without_parens [
    # Special forms
    alias: 1,
    alias: 2,
    case: 2,
    cond: 1,
    for: :*,
    import: 1,
    import: 2,
    quote: 1,
    quote: 2,
    receive: 1,
    require: 1,
    require: 2,
    try: 1,
    with: :*,

    # Kernel
    def: 1,
    def: 2,
    defp: 1,
    defp: 2,
    defguard: 1,
    defguardp: 1,
    defmacro: 1,
    defmacro: 2,
    defmacrop: 1,
    defmacrop: 2,
    defmodule: 2,
    defdelegate: 2,
    defexception: 1,
    defoverridable: 1,
    defstruct: 1,
    destructure: 2,
    raise: 1,
    raise: 2,
    reraise: 2,
    reraise: 3,
    if: 2,
    unless: 2,
    use: 1,
    use: 2,

    # Stdlib,
    defrecord: 2,
    defrecord: 3,
    defrecordp: 2,
    defrecordp: 3,

    # Testing
    assert: 1,
    assert: 2,
    assert_in_delta: 3,
    assert_in_delta: 4,
    assert_raise: 2,
    assert_raise: 3,
    assert_receive: 1,
    assert_receive: 2,
    assert_receive: 3,
    assert_received: 1,
    assert_received: 2,
    doctest: 1,
    doctest: 2,
    refute: 1,
    refute: 2,
    refute_in_delta: 3,
    refute_in_delta: 4,
    refute_receive: 1,
    refute_receive: 2,
    refute_receive: 3,
    refute_received: 1,
    refute_received: 2,
    setup: 1,
    setup: 2,
    setup_all: 1,
    setup_all: 2,
    test: 1,
    test: 2,

    # Mix config
    config: 2,
    config: 3,
    import_config: 1
  ]
NickNeck commented 3 months ago

Hi Andrew, This looks great. Thanks again for your work. I've left you some comments. Have a nice Sunday.

sodapopcan commented 3 months ago

Elixir has a bunch of defaults for locals without parens. We should also add these.

Oh, and looking at this I now understand :* which I did not know you can do!

When using Mix.Tasks.Format.formatter_opts_for_file("mix.exs") in my applications it returns the above list but doesn't here. I clearly need to better grok how this function works but will figure it out.

Thanks for all the feedback! I will finish this up later today.

sodapopcan commented 3 months ago

I had some time before my dog walk this morning so I've implemented your feedback! Thanks, I learned some stuff there :)

There are only a few tests but I feel they are actually pretty sufficient as they cover the main cases. Anything more feels like I'm just testing Sourceror.

sodapopcan commented 2 months ago

Thanks for merging but I was planning on adding docs! I can still do that if you like though fine by me if you'd prefer to. Very sorry for the delay I got caught up in another personal project.

NickNeck commented 2 months ago

Oh, sorry. It would be great if you want to could add some docs. I would also like to ask you if it is ok if you to rename the task to LocalsWithoutParens. I have done this alright in main, but we could change the name again.

No worries about the delay, all is well and good. We have no deadlines here.