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.

Sign up for GitHub

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

Jump to bottom

if-then-else/needless_bool is breaking easily #11248

Closed
GuillaumeGomez opened this issue Jul 28, 2023 · 4 comments
Closed

if-then-else/needless_bool is breaking easily #11248

GuillaumeGomez opened this issue Jul 28, 2023 · 4 comments
Labels
C-bug Category: Clippy is not doing the correct thing

Comments

@GuillaumeGomez
Copy link
Member

Summary

The if-then-else lint is breaking very easily.

Reproducer

if x {
    // some comment
    true
} else {
    true
}

The if-then-else lint is not triggered if the block contains comments.

Version

I tested it in clippy repository directly.

Additional Labels

No response
@GuillaumeGomez GuillaumeGomez added the C-bug Category: Clippy is not doing the correct thing label Jul 28, 2023
@y21
Copy link
Member

y21 commented Jul 28, 2023

I'm assuming you mean the needless_bool lint?
The reproducer confusingly triggers the if_same_then_else lint, which tripped me up when I was trying to test this locally and made me think there's no issue here.
Maybe you should change one of the bool literals in the example to false, so that they aren't the same code and doesn't trigger a different lint

@GuillaumeGomez
Copy link
Member Author

Indeed, I got the lint name wrong (updating the issue). Some more details: I have the issue because I'm migrating clippy ui tests to use the //~ everywhere. And in this case, it simply breaks needless_bool/simple.rs.

@GuillaumeGomez GuillaumeGomez changed the title if-then-else is breaking easily if-then-else/needless_bool is breaking easily Jul 28, 2023
@Alexendoo
Copy link
Member

This is intentional #10766

@GuillaumeGomez
Copy link
Member Author

Ah.

bors added a commit that referenced this issue Aug 22, 2023
@bors
Auto merge of #11249 - GuillaumeGomez:ui-tests-annotations, r=Centri3…
4be90d0 
…,llogiq

Add error annotations in UI tests

As discussed on [zulip](https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/Improve.20UI.20error.20checks), this PR adds missing error annotations in UI tests.

I used this script to generate them:

<details>

```python
import os

def handle_err(line, stderr, elems, kind, i):
    msg = line.split("{}: ".format(kind), 1)[1]
    i += 1
    try:
        line_nb = int(stderr[i].split(":")[1])
    except Exception:
        return i
    in_macro = False
    note_found = False
    help_found = False
    elem = {"kind": kind, "msg": msg, "line": line_nb, "notes": [], "helps": []}
    while i < len(stderr):
        if len(stderr[i]) == 0:
            break
        elif stderr[i].startswith("note:"):
            note_found = True # error checker doesn't like multi-note apparently.
        elif stderr[i].startswith("   = note:"):
            if not note_found and not help_found and "this error originates in the macro" not in stderr[i]:
                elem["notes"].append(stderr[i].split("   = note:", 1)[1].strip())
            note_found = True # error checker doesn't like multi-note apparently.
        elif stderr[i].startswith("   = help:") or stderr[i].startswith("help:"):
            help_found = True
        # elif stderr[i].startswith("help:"):
        #     if not help_found:
        #         elem["helps"].append(stderr[i].split("help:", 1)[1].strip())
        #         help_found = True # error checker doesn't like multi-help apparently.
        elif "in this macro invocation" in stderr[i]:
            in_macro = True
        i += 1
    if not in_macro:
        elems.append(elem)
    return i

def show_kind(kind):
    if kind == "error":
        return "ERROR"
    elif kind == "warning":
        return "WARNING"
    elif kind == "note":
        return "NOTE"
    return "HELP"

def generate_code_err(indent, elem, up):
    content = "{}//~{} {}: {}".format(indent, up, show_kind(elem["kind"]), elem["msg"])
    if up == "^":
        up = "|"
    for note in elem["notes"]:
        content += "\n{}//~{} {}: {}".format(indent, up, show_kind("note"), note)
    for help_msg in elem["helps"]:
        content += "\n{}//~{} {}: {}".format(indent, up, show_kind("help"), help_msg)
    return content, up

def update_content(p, content):
    TO_IGNORE = [
        "needless_bool/simple.rs", # #11248
        "crashes/ice-7868.rs", # Has errors but in another file.
        "trivially_copy_pass_by_ref.rs", # the `N` in the stderr needs to be replaced by the number
        "tests/ui/large_types_passed_by_value.rs", # the `N` in the stderr needs to be replaced by the number
    ]
    for to_ignore in TO_IGNORE:
        if p.endswith(to_ignore):
            return
    try:
        with open(p.replace(".rs", ".stderr"), "r", encoding="utf8") as f:
            stderr = f.read().split('\n')
    except Exception:
        return
    print("Updating `{}`".format(p))
    i = 0
    elems = []
    while i < len(stderr):
        line = stderr[i]
        if line.startswith("error: ") and not line.startswith("error: aborting due to"):
            i = handle_err(line, stderr, elems, "error", i)
        elif line.startswith("warning: ") and not line.endswith("warning emitted") and line.endswith("warnings emitted"):
            i = handle_err(line, stderr, elems, "warning", i)
        i += 1
    elems.sort(key=lambda e: e["line"], reverse=True)
    i = 0
    while i < len(elems):
        elem = elems[i]
        indent = ""
        c = 0
        line = content[elem["line"] - 1]
        while c < len(line) and line[c] == ' ':
            indent += " "
            c += 1
        new_content, up = generate_code_err(indent, elem, "^")
        i += 1
        while i < len(elems) and elems[i]["line"] == elem["line"]:
            elem = elems[i]
            ret = generate_code_err(indent, elem, up)
            new_content += "\n" + ret[0]
            up = ret[1]
            i += 1
        content.insert(elem["line"], new_content)
    with open(p, "w", encoding="utf8") as f:
        f.write("\n".join(content))

def check_if_contains_ui_test(p):
    if not p.endswith(".rs"):
        return
    with open(p, "r", encoding="utf8") as f:
        x = f.read()
    if "//~" not in x and "`@run-rustfix"` not in x and "`@aux-build"` not in x:
        update_content(p, x.split("\n"))

for path, subdirs, files in os.walk("tests/ui"):
    for name in files:
        check_if_contains_ui_test(os.path.join(path, name))
```

</details>

Then ran `cargo uibless`.

changelog: none
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-bug Category: Clippy is not doing the correct thing
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@Alexendoo @GuillaumeGomez @y21