Skip to content

Commit

Permalink
crunch crunch crunch
Browse files Browse the repository at this point in the history
  • Loading branch information
novaugust committed Apr 23, 2024
1 parent 3b52f49 commit b3147eb
Show file tree
Hide file tree
Showing 5 changed files with 154 additions and 107 deletions.
130 changes: 24 additions & 106 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,147 +16,65 @@ def deps do
]
end
```
@TODO put this somewhere more reasonable
**Note** Styler's only public API is its usage as a formatter plugin. While you're welcome to play with its internals,
they can and will change without that change being reflected in Styler's semantic version.

Then add `Styler` as a plugin to your `.formatter.exs` file

```elixir
[
plugins: [Styler]
# optionally: include styler configuration
# , styler: [alias_lifting_excludes: []]
]
```

And that's it! Now when you run `mix format` you'll also get the benefits of Styler's *definitely-always-right* style fixes.
And that's it! Now when you run `mix format` you'll also get the benefits of Styler's Stylish Stylings.

### Configuration

There isn't any! This is intentional.
@TODO document: config for lifting, and why we won't add options other configs

Styler is @adobe's internal Style Guide Enforcer - allowing exceptions to the styles goes against that ethos. Happily, it's open source and thus yours to do with as you will =)

## Features (or as we call them, "Styles")

At this point, Styler does a lot. We've catalogued a list of Credo rules that it automatically fixes, but it does some things -
like shrinking function heads down to a single line when possible - that Credo doesn't care about.
@TODO link examples

Ultimately, the best way to see what Styler does is to just try it out! What could go wrong? (You're using version control, right?)
## Styler & Credo

### Credo Rules Styler Replaces

If you're using Credo and Styler, **we recommend disabling these rules in `.credo.exs`** to save on unnecessary checks in CI.

Disabling the rules means updating your `.credo.exs` depending on your configuration:

- if you're using `checks: %{enabled: [...]}`, ensure none of the checks are listed in your enabled checks
- if you're using `checks: %{disabled: [...]}`, copy/paste the snippet below into the list
- if you're using `checks: [...]`, copy/paste the snippet below into the list and ensure none of the checks appear earlier in the list

```elixir
# Styler Rewrites
#
# The following rules are automatically rewritten by Styler and so disabled here to save time
# Some of the rules have `priority: :high`, meaning Credo runs them unless we explicitly disable them
# (removing them from this file wouldn't be enough, the `false` is required)
#
# Some rules have a comment before them explaining ways Styler deviates from the Credo rule.
#
# always expands `A.{B, C}`
{Credo.Check.Consistency.MultiAliasImportRequireUse, false},
# including `case`, `fn` and `with` statements
{Credo.Check.Consistency.ParameterPatternMatching, false},
# Styler implements this rule with a depth of 3 and minimum repetition of 2
{Credo.Check.Design.AliasUsage, false},
{Credo.Check.Readability.AliasOrder, false},
{Credo.Check.Readability.BlockPipe, false},
# goes further than formatter - fixes bad underscores, eg: `100_00` -> `10_000`
{Credo.Check.Readability.LargeNumbers, false},
# adds `@moduledoc false`
{Credo.Check.Readability.ModuleDoc, false},
{Credo.Check.Readability.MultiAlias, false},
{Credo.Check.Readability.OneArityFunctionInPipe, false},
# removes parens
{Credo.Check.Readability.ParenthesesOnZeroArityDefs, false},
{Credo.Check.Readability.PipeIntoAnonymousFunctions, false},
{Credo.Check.Readability.PreferImplicitTry, false},
{Credo.Check.Readability.SinglePipe, false},
# **potentially breaks compilation** - see **Troubleshooting** section below
{Credo.Check.Readability.StrictModuleLayout, false},
{Credo.Check.Readability.StringSigils, false},
{Credo.Check.Readability.UnnecessaryAliasExpansion, false},
{Credo.Check.Readability.WithSingleClause, false},
{Credo.Check.Refactor.CaseTrivialMatches, false},
{Credo.Check.Refactor.CondStatements, false},
# in pipes only
{Credo.Check.Refactor.FilterCount, false},
# in pipes only
{Credo.Check.Refactor.MapInto, false},
# in pipes only
{Credo.Check.Refactor.MapJoin, false},
{Credo.Check.Refactor.NegatedConditionsInUnless, false},
{Credo.Check.Refactor.NegatedConditionsWithElse, false},
# allows ecto's `from
{Credo.Check.Refactor.PipeChainStart, false},
{Credo.Check.Refactor.RedundantWithClauseResult, false},
{Credo.Check.Refactor.UnlessWithElse, false},
{Credo.Check.Refactor.WithClauses, false},
```
@TODO link credo doc

## Your first Styling

**Speed**: Expect the first run to take some time as `Styler` rewrites violations of styles.

Once styled the first time, future styling formats shouldn't take noticeably more time.

### Troubleshooting: Compilation broke due to Module Directive rearrangement

Styler naively moves module attributes, which can break compilation. For now, the only fix is some elbow grease.
## Styler can break your code

#### Module Attribute dependency

Another common compilation break on the first run is a `@moduledoc` that depended on another module attribute which
was moved below it.

For example, given the following broken code after an initial `mix format`:

```elixir
defmodule MyGreatLibrary do
@moduledoc make_pretty_docs(@library_options)
use OptionsMagic, my_opts: @library_options

@library_options [ ... ]
end
```

You can fix the code by moving the static value outside of the module into a naked variable and then reference it in the module. (Note that `use` macros need an `unquote` wrapping the variable!)

Yes, this is a thing you can do in a `.ex` file =)

```elixir
library_options = [ ... ]

defmodule MyGreatLibrary do
@moduledoc make_pretty_docs(library_options)
use OptionsMagic, my_opts: unquote(library_options)

@library_options library_options
end
```
@TODO link troubleshooting
mention our rewrite of case true false to if and how we're OK with this being _Styler_, not _SemanticallyEquivalentRewriter_.

## Thanks & Inspiration

### [Sourceror](https://github.com/doorgan/sourceror/)

This work was inspired by earlier large-scale rewrites of an internal codebase that used the fantastic tool `Sourceror`.
Styler's first incarnation was as one-off scripts to rewrite an internal codebase to allow Credo rules to be turned on.

The initial implementation of Styler used Sourceror, but Sourceror's AST-embedding comment algorithm slows Styler down to
the point that it's no longer an appropriate drop-in for `mix format`.
These rewrites were entirely powered by the terrific `Sourceror` library.

Still, we're grateful for the inspiration Sourceror provided and the changes to the Elixir AST APIs that it drove.
While `Styler` no longer relies on `Sourceror`, we're grateful for its author's help with those scripts, the inspiration
Sourceror provided in showing us what was possible, and the changes to the Elixir AST APIs that it drove.

The AST-Zipper implementation in this project was forked from Sourceror's implementation.
Styler's [AST-Zipper](`m:Styler.Zipper`) implementation in this project was forked from Sourceror. Zipper has been a crucial
part of our ability to ergonomically zip around (heh) Elixir AST.

### [Credo](https://github.com/rrrene/credo/)

Similarly, this project originated from one-off scripts doing large scale rewrites of an enormous codebase as part of an
effort to enable particular Credo rules for that codebase. Credo's tests and implementations were referenced for implementing
Styles that took the work the rest of the way. Thanks to Credo & the Elixir community at large for coalescing around
many of these Elixir style credos.
We never would've bothered trying to rewrite our codebase if we didn't have Credo rules we wanted to apply.

Credo's tests and implementations were referenced for implementing Styles that took the work the rest of the way.

Thanks to Credo & the Elixir community at large for coalescing around many of these Elixir style credos.
59 changes: 59 additions & 0 deletions docs/credo.cheatmd
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
### Credo Rules Styler Replaces

If you're using Credo and Styler, **we recommend disabling these rules in `.credo.exs`** to save on unnecessary checks in CI.

Disabling the rules means updating your `.credo.exs` depending on your configuration:

- if you're using `checks: %{enabled: [...]}`, ensure none of the checks are listed in your enabled checks
- if you're using `checks: %{disabled: [...]}`, copy/paste the snippet below into the list
- if you're using `checks: [...]`, copy/paste the snippet below into the list and ensure none of the checks appear earlier in the list

```elixir
# Styler Rewrites
#
# The following rules are automatically rewritten by Styler and so disabled here to save time
# Some of the rules have `priority: :high`, meaning Credo runs them unless we explicitly disable them
# (removing them from this file wouldn't be enough, the `false` is required)
#
# Some rules have a comment before them explaining ways Styler deviates from the Credo rule.
#
# always expands `A.{B, C}`
{Credo.Check.Consistency.MultiAliasImportRequireUse, false},
# including `case`, `fn` and `with` statements
{Credo.Check.Consistency.ParameterPatternMatching, false},
# Styler implements this rule with a depth of 3 and minimum repetition of 2
{Credo.Check.Design.AliasUsage, false},
{Credo.Check.Readability.AliasOrder, false},
{Credo.Check.Readability.BlockPipe, false},
# goes further than formatter - fixes bad underscores, eg: `100_00` -> `10_000`
{Credo.Check.Readability.LargeNumbers, false},
# adds `@moduledoc false`
{Credo.Check.Readability.ModuleDoc, false},
{Credo.Check.Readability.MultiAlias, false},
{Credo.Check.Readability.OneArityFunctionInPipe, false},
# removes parens
{Credo.Check.Readability.ParenthesesOnZeroArityDefs, false},
{Credo.Check.Readability.PipeIntoAnonymousFunctions, false},
{Credo.Check.Readability.PreferImplicitTry, false},
{Credo.Check.Readability.SinglePipe, false},
# **potentially breaks compilation** - see **Troubleshooting** section below
{Credo.Check.Readability.StrictModuleLayout, false},
{Credo.Check.Readability.StringSigils, false},
{Credo.Check.Readability.UnnecessaryAliasExpansion, false},
{Credo.Check.Readability.WithSingleClause, false},
{Credo.Check.Refactor.CaseTrivialMatches, false},
{Credo.Check.Refactor.CondStatements, false},
# in pipes only
{Credo.Check.Refactor.FilterCount, false},
# in pipes only
{Credo.Check.Refactor.MapInto, false},
# in pipes only
{Credo.Check.Refactor.MapJoin, false},
{Credo.Check.Refactor.NegatedConditionsInUnless, false},
{Credo.Check.Refactor.NegatedConditionsWithElse, false},
# allows ecto's `from
{Credo.Check.Refactor.PipeChainStart, false},
{Credo.Check.Refactor.RedundantWithClauseResult, false},
{Credo.Check.Refactor.UnlessWithElse, false},
{Credo.Check.Refactor.WithClauses, false},
```
35 changes: 35 additions & 0 deletions docs/styles.cheatmd
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ These apply to the piped versions as well

- File.stream!(file, options, line_or_bytes) => File.stream!(file, line_or_bytes, options)

### Function Definitions

- Shrink multi-line function defs
- Put assignments on the right

## Module Directives (`use`, `import`, `alias`, `require`, ...)

## Mix Configs
Expand All @@ -61,6 +66,36 @@ Once a file is detected as a mix config, its `config/2,3` stanzas are grouped an
- sort each group according to erlang term sorting
- move all existing assignments between the config stanzas to above the stanzas (without changing their ordering)

## Control Flow Structures (aka "Blocks": `case`, `if`, `unless`, `cond`, `with`)

### `case`

- rewrite to `if` for `true/false`, `true/_`, `false/true`


### `with`

`with` great power comes a great responsibility. don't use `with` when another (simpler!) "Control Flow Structure"

- single statement `with` with `else` clauses is rewritten to `case` (which can be further rewritten to an `if`!)
- move non `<-` out of the head and into preroll or body
- fully replace with statement with normal code as
- drop redundant identity else clause `else: (error -> error)` (also more complex matches, ala `{:error, error} -> {:error, error}`)
- Credo.Check.Refactor.RedundantWithClauseResult

### `cond`
- Credo.Check.Refactor.CondStatements

### `if`/`unless`

if/unless often looks to see if the root of the statement is a "negator", defined as one of the following operators: `:!, :not, :!=, :!==`. We always try to rewrite if/unless statements to not be negated, using the inverse construct when appropriate (but we'll never write an unless with an `else`)

- repeated negators (`!!`) are removed
- negated if/unless without an `else` are inverted to unless/if (this is done recursively until 0 or 1 negations remain)
- `unless` with `else` are inverted to negated `if` statements
- negated `if` with `else` have their clauses inverted to remove the negation
- if/unless with `else: nil` is dropped as redundant

## Pipe Chains

### Pipe Start
Expand Down
34 changes: 34 additions & 0 deletions docs/troubleshooting.cheatmd
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
### Troubleshooting: Compilation broke due to Module Directive rearrangement

Styler naively moves module attributes, which can break compilation. For now, the only fix is some elbow grease.

#### Module Attribute dependency

Another common compilation break on the first run is a `@moduledoc` that depended on another module attribute which
was moved below it.

For example, given the following broken code after an initial `mix format`:

```elixir
defmodule MyGreatLibrary do
@moduledoc make_pretty_docs(@library_options)
use OptionsMagic, my_opts: @library_options

@library_options [ ... ]
end
```

You can fix the code by moving the static value outside of the module into a naked variable and then reference it in the module. (Note that `use` macros need an `unquote` wrapping the variable!)

Yes, this is a thing you can do in a `.ex` file =)

```elixir
library_options = [ ... ]

defmodule MyGreatLibrary do
@moduledoc make_pretty_docs(library_options)
use OptionsMagic, my_opts: unquote(library_options)

@library_options library_options
end
```
3 changes: 2 additions & 1 deletion mix.exs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ defmodule Styler.MixProject do
source_url: @url,
extras: [
"CHANGELOG.md": [title: "Changelog"],
"README.md": [title: "Styler"]
"README.md": [title: "Styler"],
"docs/styles.cheatmd": [title: "Example \"Styles\" (Rewrites)"]
]
]
end
Expand Down

0 comments on commit b3147eb

Please sign in to comment.