fix(const_path_join): Handle constant expressions in path joins

examples/restriction/const_path_join/Cargo.toml

@@ -13,6 +13,10 @@ crate-type = ["cdylib"]
 name = "ui"
 path = "ui/main.rs"
 
+[[example]]
+name = "ui_const_expr"
+path = "ui/const_expr.rs"
+
 [dependencies]
 clippy_utils = { workspace = true }
 

examples/restriction/const_path_join/src/lib.rs

@@ -10,7 +10,7 @@ extern crate rustc_span;
 
 use clippy_utils::{
     diagnostics::span_lint_and_sugg, is_expr_path_def_path, match_any_def_paths,
-    source::snippet_opt,
+    source::snippet_opt, visitors::is_const_evaluatable,
 };
 use dylint_internal::paths;
 use rustc_ast::LitKind;
@@ -54,37 +54,152 @@ dylint_linting::declare_late_lint! {
     "joining of constant path components"
 }
 
+#[derive(Clone, Debug)]
+enum ComponentType {
+    Literal(String),
+    Constant(String),
+    Other,
+}
+
 enum TyOrPartialSpan {
     Ty(&'static [&'static str]),
     PartialSpan(Span),
 }
 
 impl<'tcx> LateLintPass<'tcx> for ConstPathJoin {
-    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'_>) {
+    #[allow(clippy::too_many_lines)]
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
         let (components, ty_or_partial_span) = collect_components(cx, expr);
         if components.len() < 2 {
             return;
         }
-        let path = components.join("/");
-        let (span, sugg) = match ty_or_partial_span {
-            TyOrPartialSpan::Ty(ty) => (expr.span, format!(r#"{}::from("{path}")"#, ty.join("::"))),
-            TyOrPartialSpan::PartialSpan(partial_span) => {
-                (partial_span, format!(r#".join("{path}")"#))
-            }
-        };
-        span_lint_and_sugg(
-            cx,
-            CONST_PATH_JOIN,
-            span,
-            "path could be constructed from a string literal",
-            "use",
-            sugg,
-            Applicability::MachineApplicable,
-        );
+
+        let all_literals = components
+            .iter()
+            .all(|c| matches!(c, ComponentType::Literal(_)));
+
+        let all_const_or_literal = components
+            .iter()
+            .all(|c| !matches!(c, ComponentType::Other));
+
+        // Only consider constants if there are multiple constants
+        // If there's just one constant with string literals, treat as normal joins
+        let has_multiple_constants = components
+            .iter()
+            .filter(|c| matches!(c, ComponentType::Constant(_)))
+            .count() > 1;
+
+        if all_literals {
+            // Suggest joining into a single string literal
+            let joined_path = components
+                .iter()
+                .map(|c| match c {
+                    ComponentType::Literal(s) => s.as_str(),
+                    _ => unreachable!(), // Already checked all are literals
+                })
+                .collect::<Vec<&str>>()
+                .join("/");
+
+            let (span, sugg) = match ty_or_partial_span {
+                TyOrPartialSpan::Ty(ty) => (
+                    expr.span,
+                    format!(r#"{}::from("{joined_path}")"#, ty.join("::"))
+                ),
+                TyOrPartialSpan::PartialSpan(partial_span) => {
+                    (partial_span, format!(r#".join("{joined_path}")"#))
+                }
+            };
+
+            span_lint_and_sugg(
+                cx,
+                CONST_PATH_JOIN,
+                span,
+                "path could be constructed from a string literal",
+                "use",
+                sugg,
+                Applicability::MachineApplicable,
+            );
+        } else if all_const_or_literal && has_multiple_constants {
+            // Suggest using concat!() only when there are multiple constant expressions
+            let concat_args = components
+                .iter()
+                .enumerate()
+                .flat_map(|(i, c)| {
+                    let mut items = Vec::new();
+                    match c {
+                        ComponentType::Literal(s) => items.push(format!(r#""{s}""#)),
+                        ComponentType::Constant(s) => items.push(s.clone()),
+                        ComponentType::Other => unreachable!(),
+                    }
+                    // Add path separator, except for the last component
+                    if i < components.len() - 1 {
+                        items.push(r#""/""#.to_string());
+                    }
+                    items
+                })
+                .collect::<Vec<String>>()
+                .join(", ");
+
+            let (span, sugg) = match ty_or_partial_span {
+                TyOrPartialSpan::Ty(ty) => (
+                    expr.span,
+                    format!(r"{}::from(concat!({concat_args}))", ty.join("::"))
+                ),
+                TyOrPartialSpan::PartialSpan(partial_span) => {
+                    // We need to replace the entire chain of joins with a single join of the concat
+                    let full_span = expr.span.with_lo(partial_span.lo());
+                    (full_span, format!(r".join(concat!({concat_args}))"))
+                }
+            };
+
+            span_lint_and_sugg(
+                cx,
+                CONST_PATH_JOIN,
+                span, 
+                "path could be constructed with concat!",
+                "consider using",
+                sugg,
+                Applicability::MachineApplicable,
+            );
+        } else if all_const_or_literal {
+            // For a single constant expression with string literals, treat like string literals
+            let joined_path = components
+                .iter()
+                .map(|c| match c {
+                    ComponentType::Literal(s) => s.as_str(),
+                    ComponentType::Constant(_) => "..", // We know it's a path component
+                    ComponentType::Other => unreachable!(), // Already checked all are const or literals
+                })
+                .collect::<Vec<&str>>()
+                .join("/");
+
+            let (span, sugg) = match ty_or_partial_span {
+                TyOrPartialSpan::Ty(ty) => (
+                    expr.span,
+                    format!(r#"{}::from("{joined_path}")"#, ty.join("::"))
+                ),
+                TyOrPartialSpan::PartialSpan(partial_span) => {
+                    // Use a wider span to replace all the joins
+                    let full_span = expr.span.with_lo(partial_span.lo());
+                    (full_span, format!(r#".join("{joined_path}")"#))
+                }
+            };
+
+            span_lint_and_sugg(
+                cx,
+                CONST_PATH_JOIN,
+                span,
+                "path could be constructed from a string literal",
+                "use",
+                sugg,
+                Applicability::MachineApplicable,
+            );
+        }
+        // If it contains ComponentType::Other, do nothing
     }
 }
 
-fn collect_components(cx: &LateContext<'_>, mut expr: &Expr<'_>) -> (Vec<String>, TyOrPartialSpan) {
+fn collect_components<'tcx>(cx: &LateContext<'tcx>, mut expr: &'tcx Expr<'tcx>) -> (Vec<ComponentType>, TyOrPartialSpan) {
     let mut components_reversed = Vec::new();
     let mut partial_span = expr.span.with_lo(expr.span.hi());
 
@@ -97,10 +212,11 @@ fn collect_components(cx: &LateContext<'_>, mut expr: &Expr<'_>) -> (Vec<String>
                 &[&paths::CAMINO_UTF8_PATH_JOIN, &paths::PATH_JOIN],
             )
             .is_some()
-            && let Some(s) = is_lit_string(cx, arg)
+            && let component_type = check_component_type(cx, arg)
+            && !matches!(component_type, ComponentType::Other)
         {
             expr = receiver;
-            components_reversed.push(s);
+            components_reversed.push(component_type);
             partial_span = partial_span.with_lo(receiver.span.hi());
             continue;
         }
@@ -112,9 +228,10 @@ fn collect_components(cx: &LateContext<'_>, mut expr: &Expr<'_>) -> (Vec<String>
         && (is_expr_path_def_path(cx, callee, &paths::CAMINO_UTF8_PATH_NEW)
             || is_expr_path_def_path(cx, callee, &paths::PATH_NEW)
             || ty.is_some())
-        && let Some(s) = is_lit_string(cx, arg)
+        && let component_type = check_component_type(cx, arg)
+        && !matches!(component_type, ComponentType::Other)
     {
-        components_reversed.push(s);
+        components_reversed.push(component_type);
         TyOrPartialSpan::Ty(ty.unwrap_or_else(|| {
             if is_expr_path_def_path(cx, callee, &paths::CAMINO_UTF8_PATH_NEW) {
                 &paths::CAMINO_UTF8_PATH_BUF
@@ -156,9 +273,6 @@ fn is_lit_string(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<String> {
     if !expr.span.from_expansion()
         && let ExprKind::Lit(lit) = &expr.kind
         && let LitKind::Str(symbol, _) = lit.node
-        // smoelius: I don't think the next line should be necessary. But following the upgrade to
-        // nightly-2023-08-24, `expr.span.from_expansion()` above started returning false for
-        // `env!(...)`.
         && snippet_opt(cx, expr.span) == Some(format!(r#""{}""#, symbol.as_str()))
     {
         Some(symbol.to_ident_string())
@@ -167,7 +281,36 @@ fn is_lit_string(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<String> {
     }
 }
 
+fn check_component_type<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> ComponentType {
+    // Check if it's a direct string literal
+    if let Some(s) = is_lit_string(cx, expr) {
+        return ComponentType::Literal(s);
+    }
+
+    // Check if it's a concat!() or env!() macro
+    if let ExprKind::Call(callee, _) = expr.kind {
+        if is_expr_path_def_path(cx, callee, &["std", "concat"]) || 
+           is_expr_path_def_path(cx, callee, &["std", "env"]) {
+            if let Some(snippet) = snippet_opt(cx, expr.span) {
+                return ComponentType::Constant(snippet);
+            }
+        }
+    }
+
+    // Check if it's a constant-evaluatable string expression
+    if !expr.span.from_expansion()
+        && is_const_evaluatable(cx, expr)
+        && matches!(cx.typeck_results().expr_ty(expr).kind(), ty::Str)
+    {
+        if let Some(snippet) = snippet_opt(cx, expr.span) {
+            return ComponentType::Constant(snippet);
+        }
+    }
+
+    ComponentType::Other
+}
+
 #[test]
 fn ui() {
-    dylint_testing::ui_test_example(env!("CARGO_PKG_NAME"), "ui");
+    dylint_testing::ui_test_examples(env!("CARGO_PKG_NAME"));
 }

examples/restriction/const_path_join/ui/const_expr.fixed

@@ -0,0 +1,16 @@
+// run-rustfix
+
+fn main() {
+    // Test env! expressions
+    let _ = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("src/lib.rs");
+
+    // Test concat! expressions
+    let _ = std::path::PathBuf::from(concat!("path", "/to/file")).join("filename.txt");
+
+    // Test nested expressions
+    let dir = env!("CARGO_MANIFEST_DIR");
+    let _ = std::path::PathBuf::from(dir).join("tests/fixtures");
+
+    // Test with camino paths
+    let _ = camino::Utf8PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("src");
+} 

examples/restriction/const_path_join/ui/const_expr.rs

@@ -0,0 +1,16 @@
+// run-rustfix
+
+fn main() {
+    // Test env! expressions
+    let _ = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("src").join("lib.rs");
+
+    // Test concat! expressions
+    let _ = std::path::PathBuf::from(concat!("path", "/to/file")).join("filename.txt");
+
+    // Test nested expressions
+    let dir = env!("CARGO_MANIFEST_DIR");
+    let _ = std::path::PathBuf::from(dir).join("tests").join("fixtures");
+
+    // Test with camino paths
+    let _ = camino::Utf8PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("src");
+} 

examples/restriction/const_path_join/ui/const_expr.stderr

@@ -0,0 +1,16 @@
+warning: path could be constructed from a string literal
+  --> $DIR/const_expr.rs:5:65
+   |
+LL |     let _ = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("src").join("lib.rs");
+   |                                                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `.join("src/lib.rs")`
+   |
+   = note: `#[warn(const_path_join)]` on by default
+
+warning: path could be constructed from a string literal
+  --> $DIR/const_expr.rs:12:42
+   |
+LL |     let _ = std::path::PathBuf::from(dir).join("tests").join("fixtures");
+   |                                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `.join("tests/fixtures")`
+
+warning: 2 warnings emitted
+