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

bevy::main_return_without_appexit is emitted when AppExit is stored in a variable #87

Open
BD103 opened this issue Sep 18, 2024 · 3 comments · May be fixed by #184
Open

bevy::main_return_without_appexit is emitted when AppExit is stored in a variable #87

BD103 opened this issue Sep 18, 2024 · 3 comments · May be fixed by #184
Labels
A-Linter Related to the linter and custom lints C-Bug A bug in the program

Comments

@BD103
Copy link
Member

BD103 commented Sep 18, 2024

fn main() {
    let app_exit = App::new().run();
    println!("{app_exit:?}");
}

The above causes a warning, though it probably shouldn't. While the intended solution is to return AppExit, there are multiple others. We mainly just want AppExit to be handled, so Apps do not silently fail.

@BD103 BD103 added A-Linter Related to the linter and custom lints C-Bug A bug in the program labels Sep 18, 2024
@BD103 BD103 added this to the `bevy_lint` 0.1.0 milestone Sep 25, 2024
@BD103
Copy link
Member Author

BD103 commented Sep 27, 2024

This can be fixed by implementing the check_local() method, which gives access to the Expr through LetStmt::init.

@BD103
Copy link
Member Author

BD103 commented Oct 22, 2024

This could be solved by is_expr_used_or_unified(). We could probably rename this lint to the more generic unused_appexit.

@BD103
Copy link
Member Author

BD103 commented Oct 31, 2024

I began some work in the unused-appexit branch. The code is surprisingly simple!

impl<'tcx> LateLintPass<'tcx> for UnusedAppExit {
    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
        // Find a method call that matches `.run()`.
        if let ExprKind::MethodCall(path, src, _, method_span) = expr.kind
            && path.ident.name == self.run_symbol
        {
            // Get the type of `src` for `src.run()`. We peel away all references because
            // both `App` and `&mut App` are allowed.
            let ty = cx.typeck_results().expr_ty(src).peel_refs();

            // If `src` is a Bevy `App` and the returned `AppExit` is not used, emit the lint.
            if match_type(cx, ty, &crate::paths::APP) && !is_expr_used_or_unified(cx.tcx, expr) {
                span_lint(
                    cx,
                    UNUSED_APPEXIT.lint,
                    method_span,
                    UNUSED_APPEXIT.lint.desc,
                );
            }
        }
    }
}

Either way, I'm going to push this issue back until 0.2.0. main_return_without_appexit is pedantic, so many won't even use it, and this specific bug is unlikely to be hit that often.

@BD103 BD103 linked a pull request Nov 26, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Linter Related to the linter and custom lints C-Bug A bug in the program
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant