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

extract alias usage #135

Merged
merged 12 commits into from
Mar 17, 2024
Merged

extract alias usage #135

merged 12 commits into from
Mar 17, 2024

Conversation

novaugust
Copy link
Contributor

@novaugust novaugust commented Mar 11, 2024

Write a Styler style that does the work of Credo.Check.Design.AliasUsage, lifting deeply nested aliases by aliasing them.

Notably, even if you're running credo's AliasUsage, styler will fix a lot more things.
This is because styler is fixing things that credo ignores:

  1. credo ignores @module_attribute A.B.C.f() due to a mistaken report that dialyzer can't understand aliases
  2. credo only looks for deep modules being invoked. apply(A.B.C, :f, []) doesn't set credo off. (credo does complain if the module is the only argument to a function call)

Yes, I thought of at least some of the cases where it shouldn't do that and implemented safeguards :)

🚢 :shipit:

pretty sure this really will need configuration options to add more excluded namespaces than the stdlib (Plug for instance), but i'm merging to main as is and will do that as its own followup since it touches Styler itself

  • FUTURE WORK add a configuration option to prevent conflict with libraries styler: [alias_lifting_exclude: [Plug, GetText, ...]]
  • rewrite all aliases? Credo only warns on function calls, or if the alias is the only argument to a function call - the intention was to warn on any alias being used as an arg (... why arg?). i had all aliases rewriting for a bit, but you maybe need to carve out exemptions (dont go into quote blocks)
  • dont go into quote blocks ?
  • don't extract aliases with unquotes in them
  • assert new aliases don't move comments
  • there's a bug where submodules can hit conflicts

Stream String StringIO Supervisor System Task Time
Tuple URI Version)a)

@libraries MapSet.new(~w(Ecto Plug Phoenix Oban)a)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

going to have to make this configurable probably 🤔

@@ -0,0 +1,110 @@
# Copyright 2023 Adobe. All rights reserved.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

welcome to 2024!

@novaugust novaugust marked this pull request as ready for review March 17, 2024 00:45
@novaugust novaugust merged commit 661a44d into main Mar 17, 2024
5 checks passed
@novaugust novaugust deleted the me/alias-usage branch March 17, 2024 00:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant