Comments (7)
@philipgiuliani @mgwidmann Sorry, I forgot to ping you. This is part of v0.4.0-beta1
.
from credo.
I should really improve the explain
text for that check. 😕
The reasoning is that pattern matching in a condition will make the code harder to reason about, especially when maintained by a different person than the original author. Assigning in a condition can also be mistaken for a comparison (e.g. when working under deadline pressure).
Example:
if {:ok, %{"error" => true, "message" => message}} = parameter1 do
do_error_handling
end
In more complicated cases like this, the code might be more maintainable in the future if it reads like this:
case parameter1 do
{:ok, %{"error" => true, "message" => message}} -> do_error_handling
_ -> nil
end
I am guilty of writing code like this in the past, as I thought it to be clever and less verbose than the alternative. But the path to hell is paved with clever solutions and good intentions.
That said, nothing Credo suggests is set in stone. Maybe we should have a configuration option to allow simple assignments like the one in your example?
from credo.
I believe your example is no longer a conditional. Meaning, if the match fails it won't execute the code either way, thats certainly worth warning about.
However, in the most common use case of the match operator is actually a presence check.
if parameter = get_parameter_data() do
do_something_with(parameter)
end
Which is just shorthand for
parameter = get_parameter_data()
if parameter do
do_something_with(parameter)
end
In my experience, this is ok, as long as the match is entirely a wildcard (only a variable). I believe this error message should only trigger for "complex" matches, basically anything except a variable inside a conditional.
from credo.
Wanted to point out this is done in the Elixir codebase: https://github.com/elixir-lang/elixir/blob/574c6db3f8566a941b55c66499078d61661d08c6/lib/elixir/lib/kernel/cli.ex#L437-L448 and I'm sure in other places as well.
I would vote for a warning about any "complex" (or non-wildcard) type matches. Simply capturing the return value shouldn't be a problem unless it's the same name as another local variable/parameter (since that would then be bad if it changed to a ==)
from credo.
I heard you 😉 The behaviour will change like we discussed above.
Development is a bit slow at the moment, because there is a lot of stuff happening at my day job AND I am giving a talk about Credo at next week's ElixirConf.EU in Berlin and the preparation is taking up most of my free time. 😁
from credo.
Awesome! Just looked for the same thing. Its possible to use 0.4.0 already now?
PS: Your talk was amazing @rrrene ! That's why im using Credo now... :)
from credo.
I will release a beta this evening. Still working on the "create your own checks" UI, but most of the other stuff is done.
Will ping you here when it is released! 👍
from credo.
Related Issues (20)
- Sarif output cannot be parsed by `github/codeql-action/upload-sarif@v2` HOT 4
- False positive: Use a pipeline when there are nested function calls HOT 2
- Compilation error in file lib/credo/check/consistency/exception_names.ex
- `Credo.Check.Warning.MissedMetadataKeyInLoggerConfig` should by default allow Erlang's metadata keys
- Include both start and end line_no and column in issue
- show allow TODO Tag
- Adding `uniq` to dependencies breaks null-ls in neovim HOT 2
- Error while running Elixir.Credo.Check.Warning.MissedMetadataKeyInLoggerConfig
- Analyzing a single file through stdin is slow when individual check configs use `:excluded` option HOT 1
- False positive NestedFunctionCalls report when interpolating strings HOT 1
- NegatedIsNil check can consume all available memory
- Slow credo runs since 1.15.x HOT 3
- Deprecation warning for Regex.regex?/1 HOT 2
- Changing color scheme in vs code HOT 2
- [Bug]credo compilation failure HOT 2
- Check running by default? HOT 1
- Question around .credo.exs merge HOT 9
- Credo fails on elixir 1.15.4 on bad Keyword.drop usage HOT 2
- module dependency check errors HOT 3
- Custom check shipped with lib not loading HOT 14
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from credo.