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

New rule: NoUnreachableCode/NoUnreachableNestedCode #30

Closed
jfmengels opened this issue Sep 9, 2020 · 2 comments
Closed

New rule: NoUnreachableCode/NoUnreachableNestedCode #30

jfmengels opened this issue Sep 9, 2020 · 2 comments
Labels
help wanted Extra attention is needed

Comments

@jfmengels
Copy link
Owner

jfmengels commented Sep 9, 2020

What the rule should do:

The rule should detect unreachable branches. These unreachable branches contain dead code, and therefore logically unused code.

What problems does it solve:

Unreachable branches are an indicator of either:

  • Error: errors in logical thinking, which the user should be made aware of
  • Error: Usage of the wrong variable used instead of another of the same type
  • work in progress that should be finished
  • Simplification: code that could be simplified

Example of things the rule would report:

if (value == stringLiteral) then
  if (value != stringLiteral) then
    -- you can prove that this is dead code
  else
    -- nested else
else
  -- outer else

Example of things the rule would not report:

if (value == stringLiteral) then
  -- nested else
else
  -- outer else

When (not) to enable this rule:

???

Questions:

  • Should this rule also warn about cases where the condition can be evaluated statically (like if True then or if a == a then), or should this be the task of another rule? (I'm leaning towards a different rule, but maybe in a different package.

  • Do you see a more fitting name for the rule?

  • Can you think of reasons not to have an automatic fix?

  • Should the rule handle case expressions too?

case value of
  Foo ->
	case value of
		Foo ->
            1
        Bar ->
            2 -- dead branch, we know value == Foo
  Bar -> 2

I am wondering whether this will not be annoying in cases where you'll need to destructure again, but I think that's unwarranted concern 🤔

I think the first iteration of the rule can concentrate on handling if expressions only anyway.

@jfmengels jfmengels changed the title New rule: NoUnreachableNestedCode New rule: NoUnreachableCode/NoUnreachableNestedCode Sep 9, 2020
@jfmengels jfmengels added help wanted Extra attention is needed hacktoberfest labels Sep 25, 2020
@jfmengels jfmengels transferred this issue from jfmengels/elm-review-unused Jul 22, 2022
@jfmengels
Copy link
Owner Author

I moved this from elm-review-unused. I think this package now covers more of the logic behind this.

What I'm slightly worried about is whether the rule will be able to add an automatic fix in all cases, because that's quite important for Simplify.

@lue-bird
Copy link
Collaborator

lue-bird commented Oct 2, 2023

For if, this is already implemented, right? And #229 covers the combination with case..of.
Should we close this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants