check against alternative method syntax

[DaAlbrecht]

Closes #94

• panicking_methods
• main_return_without_appexit
• insert_event_resource
• insert_unit_bundle

[BD103]

Thanks for starting this! Since many lints suffer from this issue, I adapted your code into a utility that we can use across the entire project:

Show Code (It's pretty long c:)

use rustc_hir::{
    def::{DefKind, Res},
    Expr, ExprKind, Path, PathSegment, QPath,
};
use rustc_lint::LateContext;
use rustc_span::{Span, Symbol};

/// An abstraction over method calls that supports both `receiver.method(args)` and
/// `Struct::method(&receiver, args)`.
///
/// # Examples
///
/// ```ignore
/// fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
///     // Don't use this, it doesn't match qualified method calls!
///     if let ExprKind::MethodCall(_, _, _, span) = expr.kind {
///         // ...
///     }
///
///     // Instead use:
///     if let Some(MethodCall { span, .. }) = MethodCall::try_from(cx, expr) {
///         // ...
///     }
/// }
/// ```
///
/// # Limitations
///
/// This does not support qualified method calls where the function is not a path. For example:
///
/// ```
/// struct Foo;
///
/// impl Foo {
///     fn bar(&self) {}
/// }
///
/// // A closure that returns a function.
/// let closure_closure = || Foo::bar;
///
/// // This will not be matched, because `closure_closure()` is an `ExprKind::Call` instead of an
/// // `ExprKind::Path`.
/// (closure_closure())(&Foo);
///
/// // This *will* be matched, though, because `Foo::bar` is an `ExprKind::Path`.
/// Foo::bar(&Foo);
/// ```
///
/// Furthermore, this does not support [language items]. If [`Self::try_from()`] encounters a
/// qualified method call that is a lang item, it will still return [`None`].
///
/// [language items]: https://rustc-dev-guide.rust-lang.org/lang-items.html
#[derive(Debug)]
pub struct MethodCall<'tcx> {
    /// The path to the method.
    ///
    /// This can be used to find the name of the method, its [`DefId`](rustc_hir::def_id::DefId),
    /// and its generic arguments.
    ///
    /// # Example
    ///
    /// ```ignore
    /// receiver.method(args);
    /// //       ^^^^^^
    ///
    /// Struct::method(&receiver, args);
    /// //      ^^^^^^
    /// ```
    pub method_path: &'tcx PathSegment<'tcx>,

    /// The receiver, or the object, of the method.
    ///
    /// This can be used to find what type the method is implemented for.
    ///
    /// TODO(BD103): Does this include the `&` reference? Should we suggest stripping it?
    ///
    /// # Example
    ///
    /// ```ignore
    /// receiver.method(args);
    /// //^^^^^^
    ///
    /// Struct::method(&receiver, args);
    /// //             ^^^^^^^^^
    /// ```
    pub receiver: &'tcx Expr<'tcx>,

    /// The arguments passed to the method.
    ///
    /// # Example
    ///
    /// ```ignore
    /// receiver.method(args);
    /// //              ^^^^
    ///
    /// Struct::method(&receiver, args);
    /// //                        ^^^^
    /// ```
    pub args: &'tcx [Expr<'tcx>],

    /// The span of the method and its arguments.
    ///
    /// This will not include the receiver if this is not a qualified method call.
    ///
    /// # Example
    ///
    /// ```ignore
    /// receiver.method(args);
    /// //       ^^^^^^^^^^^^
    ///
    /// Struct::method(&receiver, args);
    /// //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    /// ```
    pub span: Span,
}

impl<'tcx> MethodCall<'tcx> {
    /// Tries to return a [`MethodCall`] from an [`Expr`].
    ///
    /// Please see the [structure documentation](MethodCall) for examples and limitations.
    pub fn try_from(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option<Self> {
        match expr.kind {
            ExprKind::MethodCall(method_path, receiver, args, span) => Some(Self {
                method_path,
                receiver,
                args,
                span,
            }),
            ExprKind::Call(
                // We only want function calls where the function is a path, so we can use
                // `LateContext::qpath_res()`. This elimantes code where the function is the result
                // of another function, such as:
                //
                // ```
                // let closure_closure = || || {};
                //
                // // This entire expression will not be matched, though the inner
                // // `closure_closure()` will because `closure_closure` is a path.
                // (closure_closure())();
                // ```
                path @ Expr {
                    kind: ExprKind::Path(qpath),
                    ..
                },
                args,
            ) => {
                // Resolve the path, filtering out any paths that are not to associated functions.
                // This eliminates prevents us from matching standalone functions, such as:
                //
                // ```
                // fn function() {}
                //
                // // This will not be matched, since `function()`'s definition is not within an
                // `impl` block.
                // function();
                // ```
                if let Res::Def(DefKind::AssocFn, def_id) = cx.qpath_res(qpath, path.hir_id) {
                    // Retrieve the identifiers for all the arguments to this function.
                    let inputs = cx.tcx.fn_arg_names(def_id);

                    // If the name of the first argument is `self`, then it *must* be a method.
                    // `self` is a reserved keyword, and cannot be used as a general function
                    // argument name.
                    if inputs
                        .first()
                        .is_some_and(|ident| ident.name == Symbol::intern("self"))
                    {
                        let method_path = match *qpath {
                            // If the qualified path is resolved, the method path must be the final
                            // segment.
                            QPath::Resolved(
                                _,
                                Path {
                                    // Match the final segment as the method path.
                                    segments: [.., method_path],
                                    ..
                                },
                            ) => method_path,
                            QPath::Resolved(_, path @ Path { segments: [], .. }) => unreachable!(
                                "found a function call path with no segments at {:?}",
                                path.span
                            ),
                            QPath::TypeRelative(_, method_path) => method_path,
                            // Lang items are not supported.
                            QPath::LangItem(_, _) => return None,
                        };

                        // Match the first argument as `receiver`, then group the rest into the
                        // slice `args`.
                        let [receiver, args @ ..] = args else {
                            // This can only happen if `args == &[]`, which shouldn't be possible,
                            // since we previously ensured that the the first element to `args`
                            // existed and was `self`.
                            unreachable!("arguments to function call was empty, even though `self` was expected, at {:?}", expr.span);
                        };

                        return Some(Self {
                            method_path,
                            receiver,
                            args,
                            span: expr.span,
                        });
                    }
                }

                // If any of the above checks fail, return `None`, as it's not a qualified method
                // call.
                None
            }
            _ => None,
        }
    }
}

With that, could you migrate all of our code to use it? The doc-comments I wrote should help! (If you don't have time, I can do this as well.)

File that I used to test the above code, you can ignore this :)

trait Trait {
    fn trait_function();
    fn trait_method(&self) {
        Self::trait_function();
    }
}

struct Struct {
    _field: usize,
}

impl Struct {
    fn inherent_function() {}
    fn inherent_method(&self) {}
}

impl Trait for Struct {
    fn trait_function() {}
    fn trait_method(&self) {}
}

fn normal_function() {}

fn main() {
    let closure = || {};
    let closure_closure = || || {};

    let struct_ = Struct { _field: 0 };

    struct_.inherent_method();
    struct_.trait_method();

    // Type relative path, assosc-fn def, def assoc-fn res
    Struct::inherent_function();

    // Type relative path, assoc-fn def, def assoc-fn res
    Struct::inherent_method(&struct_);

    // Assoc-fn def
    Struct::trait_function();

    // Type relative path, assoc-fn def, def assoc-fn res
    Struct::trait_method(&struct_);

    // Assoc-fn def
    <Struct as Trait>::trait_function();

    // Resolved path, no def, def assoc-fn res
    Trait::trait_method(&struct_);

    // Resolved path, no def, def fn res
    normal_function();

    // Resolved path, no def, local res
    (closure)();

    // Call kind, assoc-fn def, error
    (closure_closure)()();

    let x = || Struct::inherent_method;
    (x())(&struct_);
}

[DaAlbrecht]

Thanks for starting this! Since many lints suffer from this issue, I adapted your code into a utility that we can use across the entire project:

Show Code (It's pretty long c:)

use rustc_hir::{

    def::{DefKind, Res},

    Expr, ExprKind, Path, PathSegment, QPath,

};

use rustc_lint::LateContext;

use rustc_span::{Span, Symbol};



/// An abstraction over method calls that supports both `receiver.method(args)` and

/// `Struct::method(&receiver, args)`.

///

/// # Examples

///

/// ```ignore

/// fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {

///     // Don't use this, it doesn't match qualified method calls!

///     if let ExprKind::MethodCall(_, _, _, span) = expr.kind {

///         // ...

///     }

///

///     // Instead use:

///     if let Some(MethodCall { span, .. }) = MethodCall::try_from(cx, expr) {

///         // ...

///     }

/// }

/// ```

///

/// # Limitations

///

/// This does not support qualified method calls where the function is not a path. For example:

///

/// ```

/// struct Foo;

///

/// impl Foo {

///     fn bar(&self) {}

/// }

///

/// // A closure that returns a function.

/// let closure_closure = || Foo::bar;

///

/// // This will not be matched, because `closure_closure()` is an `ExprKind::Call` instead of an

/// // `ExprKind::Path`.

/// (closure_closure())(&Foo);

///

/// // This *will* be matched, though, because `Foo::bar` is an `ExprKind::Path`.

/// Foo::bar(&Foo);

/// ```

///

/// Furthermore, this does not support [language items]. If [`Self::try_from()`] encounters a

/// qualified method call that is a lang item, it will still return [`None`].

///

/// [language items]: https://rustc-dev-guide.rust-lang.org/lang-items.html

#[derive(Debug)]

pub struct MethodCall<'tcx> {

    /// The path to the method.

    ///

    /// This can be used to find the name of the method, its [`DefId`](rustc_hir::def_id::DefId),

    /// and its generic arguments.

    ///

    /// # Example

    ///

    /// ```ignore

    /// receiver.method(args);

    /// //       ^^^^^^

    ///

    /// Struct::method(&receiver, args);

    /// //      ^^^^^^

    /// ```

    pub method_path: &'tcx PathSegment<'tcx>,



    /// The receiver, or the object, of the method.

    ///

    /// This can be used to find what type the method is implemented for.

    ///

    /// TODO(BD103): Does this include the `&` reference? Should we suggest stripping it?

    ///

    /// # Example

    ///

    /// ```ignore

    /// receiver.method(args);

    /// //^^^^^^

    ///

    /// Struct::method(&receiver, args);

    /// //             ^^^^^^^^^

    /// ```

    pub receiver: &'tcx Expr<'tcx>,



    /// The arguments passed to the method.

    ///

    /// # Example

    ///

    /// ```ignore

    /// receiver.method(args);

    /// //              ^^^^

    ///

    /// Struct::method(&receiver, args);

    /// //                        ^^^^

    /// ```

    pub args: &'tcx [Expr<'tcx>],



    /// The span of the method and its arguments.

    ///

    /// This will not include the receiver if this is not a qualified method call.

    ///

    /// # Example

    ///

    /// ```ignore

    /// receiver.method(args);

    /// //       ^^^^^^^^^^^^

    ///

    /// Struct::method(&receiver, args);

    /// //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

    /// ```

    pub span: Span,

}



impl<'tcx> MethodCall<'tcx> {

    /// Tries to return a [`MethodCall`] from an [`Expr`].

    ///

    /// Please see the [structure documentation](MethodCall) for examples and limitations.

    pub fn try_from(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option<Self> {

        match expr.kind {

            ExprKind::MethodCall(method_path, receiver, args, span) => Some(Self {

                method_path,

                receiver,

                args,

                span,

            }),

            ExprKind::Call(

                // We only want function calls where the function is a path, so we can use

                // `LateContext::qpath_res()`. This elimantes code where the function is the result

                // of another function, such as:

                //

                // ```

                // let closure_closure = || || {};

                //

                // // This entire expression will not be matched, though the inner

                // // `closure_closure()` will because `closure_closure` is a path.

                // (closure_closure())();

                // ```

                path @ Expr {

                    kind: ExprKind::Path(qpath),

                    ..

                },

                args,

            ) => {

                // Resolve the path, filtering out any paths that are not to associated functions.

                // This eliminates prevents us from matching standalone functions, such as:

                //

                // ```

                // fn function() {}

                //

                // // This will not be matched, since `function()`'s definition is not within an

                // `impl` block.

                // function();

                // ```

                if let Res::Def(DefKind::AssocFn, def_id) = cx.qpath_res(qpath, path.hir_id) {

                    // Retrieve the identifiers for all the arguments to this function.

                    let inputs = cx.tcx.fn_arg_names(def_id);



                    // If the name of the first argument is `self`, then it *must* be a method.

                    // `self` is a reserved keyword, and cannot be used as a general function

                    // argument name.

                    if inputs

                        .first()

                        .is_some_and(|ident| ident.name == Symbol::intern("self"))

                    {

                        let method_path = match *qpath {

                            // If the qualified path is resolved, the method path must be the final

                            // segment.

                            QPath::Resolved(

                                _,

                                Path {

                                    // Match the final segment as the method path.

                                    segments: [.., method_path],

                                    ..

                                },

                            ) => method_path,

                            QPath::Resolved(_, path @ Path { segments: [], .. }) => unreachable!(

                                "found a function call path with no segments at {:?}",

                                path.span

                            ),

                            QPath::TypeRelative(_, method_path) => method_path,

                            // Lang items are not supported.

                            QPath::LangItem(_, _) => return None,

                        };



                        // Match the first argument as `receiver`, then group the rest into the

                        // slice `args`.

                        let [receiver, args @ ..] = args else {

                            // This can only happen if `args == &[]`, which shouldn't be possible,

                            // since we previously ensured that the the first element to `args`

                            // existed and was `self`.

                            unreachable!("arguments to function call was empty, even though `self` was expected, at {:?}", expr.span);

                        };



                        return Some(Self {

                            method_path,

                            receiver,

                            args,

                            span: expr.span,

                        });

                    }

                }



                // If any of the above checks fail, return `None`, as it's not a qualified method

                // call.

                None

            }

            _ => None,

        }

    }

}

With that, could you migrate all of our code to use it? The doc-comments I wrote should help! (If you don't have time, I can do this as well.)

O neat! Will do, thanks for the help!

[BD103]

Looks good! I have some nits and a few requested changes, but nothing too difficult to fix :)

[BD103]

Thank you so much! :D