From e4825290223f39647bf2782b9d4ef5f00554c7ee Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Thu, 8 Dec 2016 16:12:38 -0800 Subject: [PATCH 1/4] Fix invalid module suggestion --- src/libsyntax/parse/parser.rs | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 1baf0d1b54ce1..dd32b40554b5d 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -5363,26 +5363,33 @@ impl<'a> Parser<'a> { } let mut err = self.diagnostic().struct_span_err(id_sp, "cannot declare a new module at this location"); - let this_module = match self.directory.path.file_name() { - Some(file_name) => file_name.to_str().unwrap().to_owned(), - None => self.root_module_name.as_ref().unwrap().clone(), - }; - err.span_note(id_sp, - &format!("maybe move this module `{0}` to its own directory \ - via `{0}{1}mod.rs`", - this_module, - path::MAIN_SEPARATOR)); + if id_sp != syntax_pos::DUMMY_SP { + let full_path = self.sess.codemap().span_to_filename(id_sp); + let path = Path::new(&full_path); + let filename = path.file_stem().unwrap(); + let parent = path.parent().unwrap_or(Path::new("")) + .to_str().unwrap_or("").to_owned(); + let path = format!("{}/{}", + if parent.len() == 0 { "." } else { &parent }, + filename.to_str().unwrap_or("")); + err.span_note(id_sp, + &format!("maybe move this module `{0}` to its own directory \ + via `{0}{1}mod.rs`", + path, + path::MAIN_SEPARATOR)); + } if paths.path_exists { err.span_note(id_sp, &format!("... or maybe `use` the module `{}` instead \ of possibly redeclaring it", paths.name)); - Err(err) - } else { - Err(err) } + Err(err) } else { - paths.result.map_err(|err| self.span_fatal_err(id_sp, err)) + match paths.result { + Ok(succ) => Ok(succ), + Err(err) => Err(self.span_fatal_err(id_sp, &err.err_msg, &err.help_msg)), + } } } From bd880bc6bf62a223664467d4df2b056a9122b7e7 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 3 Jan 2017 12:19:13 +0100 Subject: [PATCH 2/4] Add tests for module suggestions --- src/libsyntax/parse/parser.rs | 43 ++++++++++++------- src/test/compile-fail/auxiliary/foo/bar.rs | 11 +++++ src/test/compile-fail/auxiliary/foo/mod.rs | 11 +++++ .../invalid-module-declaration.rs | 20 +++++++++ 4 files changed, 70 insertions(+), 15 deletions(-) create mode 100644 src/test/compile-fail/auxiliary/foo/bar.rs create mode 100644 src/test/compile-fail/auxiliary/foo/mod.rs create mode 100644 src/test/compile-fail/invalid-module-declaration.rs diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index dd32b40554b5d..ac72d21ec4271 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -58,8 +58,11 @@ use symbol::{Symbol, keywords}; use util::ThinVec; use std::collections::HashSet; -use std::{cmp, mem, slice}; -use std::path::{self, Path, PathBuf}; +use std::env; +use std::mem; +use std::path::{Path, PathBuf}; +use std::rc::Rc; +use std::slice; bitflags! { flags Restrictions: u8 { @@ -5364,19 +5367,29 @@ impl<'a> Parser<'a> { let mut err = self.diagnostic().struct_span_err(id_sp, "cannot declare a new module at this location"); if id_sp != syntax_pos::DUMMY_SP { - let full_path = self.sess.codemap().span_to_filename(id_sp); - let path = Path::new(&full_path); - let filename = path.file_stem().unwrap(); - let parent = path.parent().unwrap_or(Path::new("")) - .to_str().unwrap_or("").to_owned(); - let path = format!("{}/{}", - if parent.len() == 0 { "." } else { &parent }, - filename.to_str().unwrap_or("")); - err.span_note(id_sp, - &format!("maybe move this module `{0}` to its own directory \ - via `{0}{1}mod.rs`", - path, - path::MAIN_SEPARATOR)); + let mut src_path = PathBuf::from(self.sess.codemap().span_to_filename(id_sp)); + if let Some(stem) = src_path.clone().file_stem() { + let mut dest_path = src_path.clone(); + dest_path.set_file_name(stem); + dest_path.push("mod.rs"); + if let Ok(cur_dir) = env::current_dir() { + let tmp = if let (Ok(src_path), Ok(dest_path)) = + (Path::new(&src_path).strip_prefix(&cur_dir), + Path::new(&dest_path).strip_prefix(&cur_dir)) { + Some((src_path.to_path_buf(), dest_path.to_path_buf())) + } else { + None + }; + if let Some(tmp) = tmp { + src_path = tmp.0; + dest_path = tmp.1; + } + } + err.span_note(id_sp, + &format!("maybe move this module `{}` to its own \ + directory via `{}`", src_path.to_string_lossy(), + dest_path.to_string_lossy())); + } } if paths.path_exists { err.span_note(id_sp, diff --git a/src/test/compile-fail/auxiliary/foo/bar.rs b/src/test/compile-fail/auxiliary/foo/bar.rs new file mode 100644 index 0000000000000..4b6b4f5ebf82f --- /dev/null +++ b/src/test/compile-fail/auxiliary/foo/bar.rs @@ -0,0 +1,11 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +pub mod baz; diff --git a/src/test/compile-fail/auxiliary/foo/mod.rs b/src/test/compile-fail/auxiliary/foo/mod.rs new file mode 100644 index 0000000000000..6d77fb60a35de --- /dev/null +++ b/src/test/compile-fail/auxiliary/foo/mod.rs @@ -0,0 +1,11 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +pub mod bar; diff --git a/src/test/compile-fail/invalid-module-declaration.rs b/src/test/compile-fail/invalid-module-declaration.rs new file mode 100644 index 0000000000000..658fa0a65c47f --- /dev/null +++ b/src/test/compile-fail/invalid-module-declaration.rs @@ -0,0 +1,20 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// ignore-tidy-linelength + +// error-pattern: cannot declare a new module at this location +// error-pattern: maybe move this module `src/test/compile-fail/auxiliary/foo/bar.rs` to its own directory via `src/test/compile-fail/auxiliary/foo/bar/mod.rs` + +mod auxiliary { + mod foo; +} + +fn main() {} From b10c04472bf0969d123aa4461a7f0a1a255ac660 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 25 Feb 2017 17:08:46 +0100 Subject: [PATCH 3/4] Remove strip prefix --- src/libsyntax/parse/parser.rs | 18 ++---------------- .../compile-fail/invalid-module-declaration.rs | 2 +- 2 files changed, 3 insertions(+), 17 deletions(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index ac72d21ec4271..9df791c1c124e 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -58,7 +58,6 @@ use symbol::{Symbol, keywords}; use util::ThinVec; use std::collections::HashSet; -use std::env; use std::mem; use std::path::{Path, PathBuf}; use std::rc::Rc; @@ -5367,24 +5366,11 @@ impl<'a> Parser<'a> { let mut err = self.diagnostic().struct_span_err(id_sp, "cannot declare a new module at this location"); if id_sp != syntax_pos::DUMMY_SP { - let mut src_path = PathBuf::from(self.sess.codemap().span_to_filename(id_sp)); + let src_path = PathBuf::from(self.sess.codemap().span_to_filename(id_sp)); if let Some(stem) = src_path.clone().file_stem() { let mut dest_path = src_path.clone(); dest_path.set_file_name(stem); dest_path.push("mod.rs"); - if let Ok(cur_dir) = env::current_dir() { - let tmp = if let (Ok(src_path), Ok(dest_path)) = - (Path::new(&src_path).strip_prefix(&cur_dir), - Path::new(&dest_path).strip_prefix(&cur_dir)) { - Some((src_path.to_path_buf(), dest_path.to_path_buf())) - } else { - None - }; - if let Some(tmp) = tmp { - src_path = tmp.0; - dest_path = tmp.1; - } - } err.span_note(id_sp, &format!("maybe move this module `{}` to its own \ directory via `{}`", src_path.to_string_lossy(), @@ -5401,7 +5387,7 @@ impl<'a> Parser<'a> { } else { match paths.result { Ok(succ) => Ok(succ), - Err(err) => Err(self.span_fatal_err(id_sp, &err.err_msg, &err.help_msg)), + Err(err) => Err(self.span_fatal_err(id_sp, err)), } } } diff --git a/src/test/compile-fail/invalid-module-declaration.rs b/src/test/compile-fail/invalid-module-declaration.rs index 658fa0a65c47f..c15cfb8cc8e22 100644 --- a/src/test/compile-fail/invalid-module-declaration.rs +++ b/src/test/compile-fail/invalid-module-declaration.rs @@ -11,7 +11,7 @@ // ignore-tidy-linelength // error-pattern: cannot declare a new module at this location -// error-pattern: maybe move this module `src/test/compile-fail/auxiliary/foo/bar.rs` to its own directory via `src/test/compile-fail/auxiliary/foo/bar/mod.rs` +// error-pattern: maybe move this module mod auxiliary { mod foo; From 3f97b2a65c31a1c53c29d233d6b37b0258e4a6b2 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 24 Apr 2017 16:26:04 +0200 Subject: [PATCH 4/4] Add ui tests --- src/libsyntax/parse/parser.rs | 11 ++++------- .../auxiliary/foo/bar.rs | 0 .../auxiliary/foo/mod.rs | 0 .../invalid-module-declaration.rs | 0 .../invalid-module-declaration.stderr | 14 ++++++++++++++ 5 files changed, 18 insertions(+), 7 deletions(-) rename src/test/{compile-fail => ui/invalid-module-declaration}/auxiliary/foo/bar.rs (100%) rename src/test/{compile-fail => ui/invalid-module-declaration}/auxiliary/foo/mod.rs (100%) rename src/test/{compile-fail => ui/invalid-module-declaration}/invalid-module-declaration.rs (100%) create mode 100644 src/test/ui/invalid-module-declaration/invalid-module-declaration.stderr diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 9df791c1c124e..afdb0cc4fdb79 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -57,10 +57,10 @@ use tokenstream::{self, Delimited, ThinTokenStream, TokenTree, TokenStream}; use symbol::{Symbol, keywords}; use util::ThinVec; +use std::cmp; use std::collections::HashSet; use std::mem; -use std::path::{Path, PathBuf}; -use std::rc::Rc; +use std::path::{self, Path, PathBuf}; use std::slice; bitflags! { @@ -5367,7 +5367,7 @@ impl<'a> Parser<'a> { "cannot declare a new module at this location"); if id_sp != syntax_pos::DUMMY_SP { let src_path = PathBuf::from(self.sess.codemap().span_to_filename(id_sp)); - if let Some(stem) = src_path.clone().file_stem() { + if let Some(stem) = src_path.file_stem() { let mut dest_path = src_path.clone(); dest_path.set_file_name(stem); dest_path.push("mod.rs"); @@ -5385,10 +5385,7 @@ impl<'a> Parser<'a> { } Err(err) } else { - match paths.result { - Ok(succ) => Ok(succ), - Err(err) => Err(self.span_fatal_err(id_sp, err)), - } + paths.result.map_err(|err| self.span_fatal_err(id_sp, err)) } } diff --git a/src/test/compile-fail/auxiliary/foo/bar.rs b/src/test/ui/invalid-module-declaration/auxiliary/foo/bar.rs similarity index 100% rename from src/test/compile-fail/auxiliary/foo/bar.rs rename to src/test/ui/invalid-module-declaration/auxiliary/foo/bar.rs diff --git a/src/test/compile-fail/auxiliary/foo/mod.rs b/src/test/ui/invalid-module-declaration/auxiliary/foo/mod.rs similarity index 100% rename from src/test/compile-fail/auxiliary/foo/mod.rs rename to src/test/ui/invalid-module-declaration/auxiliary/foo/mod.rs diff --git a/src/test/compile-fail/invalid-module-declaration.rs b/src/test/ui/invalid-module-declaration/invalid-module-declaration.rs similarity index 100% rename from src/test/compile-fail/invalid-module-declaration.rs rename to src/test/ui/invalid-module-declaration/invalid-module-declaration.rs diff --git a/src/test/ui/invalid-module-declaration/invalid-module-declaration.stderr b/src/test/ui/invalid-module-declaration/invalid-module-declaration.stderr new file mode 100644 index 0000000000000..3e9b21cdb740f --- /dev/null +++ b/src/test/ui/invalid-module-declaration/invalid-module-declaration.stderr @@ -0,0 +1,14 @@ +error: cannot declare a new module at this location + --> $DIR/auxiliary/foo/bar.rs:11:9 + | +11 | pub mod baz; + | ^^^ + | +note: maybe move this module `$DIR/auxiliary/foo/bar.rs` to its own directory via `$DIR/auxiliary/foo/bar/mod.rs` + --> $DIR/auxiliary/foo/bar.rs:11:9 + | +11 | pub mod baz; + | ^^^ + +error: aborting due to previous error +