about summary refs log tree commit diff
diff options
context:
space:
mode:
authorflip1995 <hello@philkrones.com>2020-02-21 10:15:38 +0100
committerflip1995 <hello@philkrones.com>2020-02-21 11:14:19 +0100
commit4dd2252b17915a6b11b8e9ea4bc4500e80437a60 (patch)
tree1b9569d4cc1a9f45f25396c827e5241472cbea82
parentb562a519e69847d918d117dd9b729f92c7749d34 (diff)
downloadrust-4dd2252b17915a6b11b8e9ea4bc4500e80437a60.tar.gz
rust-4dd2252b17915a6b11b8e9ea4bc4500e80437a60.zip
Fix suggestion for weird formattings
-rw-r--r--clippy_lints/src/wildcard_imports.rs26
-rw-r--r--tests/ui/wildcard_imports.fixed9
-rw-r--r--tests/ui/wildcard_imports.rs10
-rw-r--r--tests/ui/wildcard_imports.stderr16
4 files changed, 54 insertions, 7 deletions
diff --git a/clippy_lints/src/wildcard_imports.rs b/clippy_lints/src/wildcard_imports.rs
index 3e8d28a2293..17e067a095d 100644
--- a/clippy_lints/src/wildcard_imports.rs
+++ b/clippy_lints/src/wildcard_imports.rs
@@ -1,4 +1,4 @@
-use crate::utils::{in_macro, snippet_with_applicability, span_lint_and_sugg};
+use crate::utils::{in_macro, snippet, snippet_with_applicability, span_lint_and_sugg};
 use if_chain::if_chain;
 use rustc_errors::Applicability;
 use rustc_hir::{
@@ -46,6 +46,9 @@ declare_clippy_lint! {
     /// **Known problems:** If macros are imported through the wildcard, this macro is not included
     /// by the suggestion and has to be added by hand.
     ///
+    /// Applying the suggestion when explicit imports of the things imported with a glob import
+    /// exist, may result in `unused_imports` warnings.
+    ///
     /// **Example:**
     ///
     /// Bad:
@@ -82,16 +85,27 @@ impl LateLintPass<'_, '_> for WildcardImports {
             if !used_imports.is_empty(); // Already handled by `unused_imports`
             then {
                 let mut applicability = Applicability::MachineApplicable;
-                let import_source = snippet_with_applicability(cx, use_path.span, "..", &mut applicability);
-                let (span, braced_glob) = if import_source.is_empty() {
+                let import_source_snippet = snippet_with_applicability(cx, use_path.span, "..", &mut applicability);
+                let (span, braced_glob) = if import_source_snippet.is_empty() {
                     // This is a `_::{_, *}` import
+                    // In this case `use_path.span` is empty and ends directly in front of the `*`,
+                    // so we need to extend it by one byte.
                     (
                         use_path.span.with_hi(use_path.span.hi() + BytePos(1)),
                         true,
                     )
                 } else {
+                    // In this case, the `use_path.span` ends right before the `::*`, so we need to
+                    // extend it up to the `*`. Since it is hard to find the `*` in weird
+                    // formattings like `use _ ::  *;`, we extend it up to, but not including the
+                    // `;`. In nested imports, like `use _::{inner::*, _}` there is no `;` and we
+                    // can just use the end of the item span
+                    let mut span = use_path.span.with_hi(item.span.hi());
+                    if snippet(cx, span, "").ends_with(';') {
+                        span = use_path.span.with_hi(item.span.hi() - BytePos(1));
+                    }
                     (
-                        use_path.span.with_hi(use_path.span.hi() + BytePos(3)),
+                        span,
                         false,
                     )
                 };
@@ -111,10 +125,10 @@ impl LateLintPass<'_, '_> for WildcardImports {
                     }
                 };
 
-                let sugg = if import_source.is_empty() {
+                let sugg = if braced_glob {
                     imports_string
                 } else {
-                    format!("{}::{}", import_source, imports_string)
+                    format!("{}::{}", import_source_snippet, imports_string)
                 };
 
                 let (lint, message) = if let Res::Def(DefKind::Enum, _) = use_path.res {
diff --git a/tests/ui/wildcard_imports.fixed b/tests/ui/wildcard_imports.fixed
index 398c3b56572..f447a92715d 100644
--- a/tests/ui/wildcard_imports.fixed
+++ b/tests/ui/wildcard_imports.fixed
@@ -145,3 +145,12 @@ fn test_reexported() {
     let _ = ExportedStruct;
     let _ = ExportedEnum::A;
 }
+
+#[rustfmt::skip]
+fn test_weird_formatting() {
+    use crate:: in_fn_test::exported;
+    use crate:: fn_mod::foo;
+
+    exported();
+    foo();
+}
diff --git a/tests/ui/wildcard_imports.rs b/tests/ui/wildcard_imports.rs
index 10ab25e02ad..3fd66763a9f 100644
--- a/tests/ui/wildcard_imports.rs
+++ b/tests/ui/wildcard_imports.rs
@@ -145,3 +145,13 @@ fn test_reexported() {
     let _ = ExportedStruct;
     let _ = ExportedEnum::A;
 }
+
+#[rustfmt::skip]
+fn test_weird_formatting() {
+    use crate:: in_fn_test::  * ;
+    use crate:: fn_mod::
+        *;
+
+    exported();
+    foo();
+}
diff --git a/tests/ui/wildcard_imports.stderr b/tests/ui/wildcard_imports.stderr
index ca300fbaad4..bebd9c1f852 100644
--- a/tests/ui/wildcard_imports.stderr
+++ b/tests/ui/wildcard_imports.stderr
@@ -78,5 +78,19 @@ error: usage of wildcard import
 LL |     use crate::in_fn_test::*;
    |         ^^^^^^^^^^^^^^^^^^^^ help: try: `crate::in_fn_test::{ExportedEnum, ExportedStruct, exported}`
 
-error: aborting due to 13 previous errors
+error: usage of wildcard import
+  --> $DIR/wildcard_imports.rs:151:9
+   |
+LL |     use crate:: in_fn_test::  * ;
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `crate:: in_fn_test::exported`
+
+error: usage of wildcard import
+  --> $DIR/wildcard_imports.rs:152:9
+   |
+LL |       use crate:: fn_mod::
+   |  _________^
+LL | |         *;
+   | |_________^ help: try: `crate:: fn_mod::foo`
+
+error: aborting due to 15 previous errors