From 4e9e091e91ea2ad8a6f45a9b20ff331d4bca7a23 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 16 May 2014 14:23:04 -0700 Subject: [PATCH] syntax: Tighten search paths for inner modules This is an implementation of RFC 16. A module can now only be loaded if the module declaring `mod name;` "owns" the current directory. A module is considered as owning its directory if it meets one of the following criteria: * It is the top-level crate file * It is a `mod.rs` file * It was loaded via `#[path]` * It was loaded via `include!` * The module was declared via an inline `mod foo { ... }` statement For example, this directory structure is now invalid // lib.rs mod foo; // foo.rs mod bar; // bar.rs; fn bar() {} With this change `foo.rs` must be renamed to `foo/mod.rs`, and `bar.rs` must be renamed to `foo/bar.rs`. This makes it clear that `bar` is a submodule of `foo`, and can only be accessed through `foo`. RFC: 0016-module-file-system-hierarchy Closes #14180 [breaking-change] --- src/libregex/{parse.rs => parse/mod.rs} | 0 src/libregex/{ => parse}/unicode.rs | 0 .../deriving/{generic.rs => generic/mod.rs} | 0 .../ext/deriving/{ => generic}/ty.rs | 0 src/libsyntax/ext/source_util.rs | 2 + src/libsyntax/parse/mod.rs | 7 ++- src/libsyntax/parse/parser.rs | 50 ++++++++++++++++--- .../compile-fail/circular_modules_main.rs | 2 +- src/test/compile-fail/mod_file_not_owning.rs | 15 ++++++ .../compile-fail/mod_file_not_owning_aux1.rs | 13 +++++ .../compile-fail/mod_file_not_owning_aux2.rs | 11 ++++ .../mod_dir_simple/load_another_mod.rs | 1 + 12 files changed, 93 insertions(+), 8 deletions(-) rename src/libregex/{parse.rs => parse/mod.rs} (100%) rename src/libregex/{ => parse}/unicode.rs (100%) rename src/libsyntax/ext/deriving/{generic.rs => generic/mod.rs} (100%) rename src/libsyntax/ext/deriving/{ => generic}/ty.rs (100%) create mode 100644 src/test/compile-fail/mod_file_not_owning.rs create mode 100644 src/test/compile-fail/mod_file_not_owning_aux1.rs create mode 100644 src/test/compile-fail/mod_file_not_owning_aux2.rs diff --git a/src/libregex/parse.rs b/src/libregex/parse/mod.rs similarity index 100% rename from src/libregex/parse.rs rename to src/libregex/parse/mod.rs diff --git a/src/libregex/unicode.rs b/src/libregex/parse/unicode.rs similarity index 100% rename from src/libregex/unicode.rs rename to src/libregex/parse/unicode.rs diff --git a/src/libsyntax/ext/deriving/generic.rs b/src/libsyntax/ext/deriving/generic/mod.rs similarity index 100% rename from src/libsyntax/ext/deriving/generic.rs rename to src/libsyntax/ext/deriving/generic/mod.rs diff --git a/src/libsyntax/ext/deriving/ty.rs b/src/libsyntax/ext/deriving/generic/ty.rs similarity index 100% rename from src/libsyntax/ext/deriving/ty.rs rename to src/libsyntax/ext/deriving/generic/ty.rs diff --git a/src/libsyntax/ext/source_util.rs b/src/libsyntax/ext/source_util.rs index 6e7e72bd2e843..6bc08741c079a 100644 --- a/src/libsyntax/ext/source_util.rs +++ b/src/libsyntax/ext/source_util.rs @@ -95,6 +95,8 @@ pub fn expand_include(cx: &mut ExtCtxt, sp: Span, tts: &[ast::TokenTree]) &res_rel_file(cx, sp, &Path::new(file)), + true, + None, sp); base::MacExpr::new(p.parse_expr()) } diff --git a/src/libsyntax/parse/mod.rs b/src/libsyntax/parse/mod.rs index d8a9f69e29342..8e139b049c590 100644 --- a/src/libsyntax/parse/mod.rs +++ b/src/libsyntax/parse/mod.rs @@ -184,8 +184,13 @@ pub fn new_parser_from_file<'a>(sess: &'a ParseSess, pub fn new_sub_parser_from_file<'a>(sess: &'a ParseSess, cfg: ast::CrateConfig, path: &Path, + owns_directory: bool, + module_name: Option, sp: Span) -> Parser<'a> { - filemap_to_parser(sess, file_to_filemap(sess, path, Some(sp)), cfg) + let mut p = filemap_to_parser(sess, file_to_filemap(sess, path, Some(sp)), cfg); + p.owns_directory = owns_directory; + p.root_module_name = module_name; + p } /// Given a filemap and config, return a parser diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 92e5f8da6aa7e..c42febcd607a1 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -313,6 +313,8 @@ pub fn Parser<'a>( obsolete_set: HashSet::new(), mod_path_stack: Vec::new(), open_braces: Vec::new(), + owns_directory: true, + root_module_name: None, } } @@ -342,6 +344,13 @@ pub struct Parser<'a> { pub mod_path_stack: Vec, /// Stack of spans of open delimiters. Used for error message. pub open_braces: Vec, + /// Flag if this parser "owns" the directory that it is currently parsing + /// in. This will affect how nested files are looked up. + pub owns_directory: bool, + /// Name of the root module this parser originated from. If `None`, then the + /// name is not known. This does not change while the parser is descending + /// into modules, and sub-parsers have new values for this name. + pub root_module_name: Option, } fn is_plain_ident_or_underscore(t: &token::Token) -> bool { @@ -4179,9 +4188,12 @@ impl<'a> Parser<'a> { self.push_mod_path(id, outer_attrs); self.expect(&token::LBRACE); let mod_inner_lo = self.span.lo; + let old_owns_directory = self.owns_directory; + self.owns_directory = true; let (inner, next) = self.parse_inner_attrs_and_next(); let m = self.parse_mod_items(token::RBRACE, next, mod_inner_lo); self.expect(&token::RBRACE); + self.owns_directory = old_owns_directory; self.pop_mod_path(); (id, ItemMod(m), Some(inner)) } @@ -4211,11 +4223,11 @@ impl<'a> Parser<'a> { prefix.pop(); let mod_path = Path::new(".").join_many(self.mod_path_stack.as_slice()); let dir_path = prefix.join(&mod_path); - let file_path = match ::attr::first_attr_value_str_by_name( + let mod_string = token::get_ident(id); + let (file_path, owns_directory) = match ::attr::first_attr_value_str_by_name( outer_attrs, "path") { - Some(d) => dir_path.join(d), + Some(d) => (dir_path.join(d), true), None => { - let mod_string = token::get_ident(id); let mod_name = mod_string.get().to_owned(); let default_path_str = mod_name + ".rs"; let secondary_path_str = mod_name + "/mod.rs"; @@ -4223,9 +4235,30 @@ impl<'a> Parser<'a> { let secondary_path = dir_path.join(secondary_path_str.as_slice()); let default_exists = default_path.exists(); let secondary_exists = secondary_path.exists(); + + if !self.owns_directory { + self.span_err(id_sp, + "cannot declare a new module at this location"); + let this_module = match self.mod_path_stack.last() { + Some(name) => name.get().to_strbuf(), + None => self.root_module_name.get_ref().clone(), + }; + self.span_note(id_sp, + format!("maybe move this module `{0}` \ + to its own directory via \ + `{0}/mod.rs`", this_module)); + if default_exists || secondary_exists { + self.span_note(id_sp, + format!("... or maybe `use` the module \ + `{}` instead of possibly \ + redeclaring it", mod_name)); + } + self.abort_if_errors(); + } + match (default_exists, secondary_exists) { - (true, false) => default_path, - (false, true) => secondary_path, + (true, false) => (default_path, false), + (false, true) => (secondary_path, true), (false, false) => { self.span_fatal(id_sp, format!("file not found for module `{}`", mod_name)); } @@ -4238,11 +4271,14 @@ impl<'a> Parser<'a> { } }; - self.eval_src_mod_from_path(file_path, id_sp) + self.eval_src_mod_from_path(file_path, owns_directory, + mod_string.get().to_strbuf(), id_sp) } fn eval_src_mod_from_path(&mut self, path: Path, + owns_directory: bool, + name: StrBuf, id_sp: Span) -> (ast::Item_, Vec ) { let mut included_mod_stack = self.sess.included_mod_stack.borrow_mut(); match included_mod_stack.iter().position(|p| *p == path) { @@ -4265,6 +4301,8 @@ impl<'a> Parser<'a> { new_sub_parser_from_file(self.sess, self.cfg.clone(), &path, + owns_directory, + Some(name), id_sp); let mod_inner_lo = p0.span.lo; let (mod_attrs, next) = p0.parse_inner_attrs_and_next(); diff --git a/src/test/compile-fail/circular_modules_main.rs b/src/test/compile-fail/circular_modules_main.rs index bc9f3247db706..7296ea655c2ca 100644 --- a/src/test/compile-fail/circular_modules_main.rs +++ b/src/test/compile-fail/circular_modules_main.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. - +#[path = "circular_modules_hello.rs"] mod circular_modules_hello; //~ERROR: circular modules pub fn hi_str() -> StrBuf { diff --git a/src/test/compile-fail/mod_file_not_owning.rs b/src/test/compile-fail/mod_file_not_owning.rs new file mode 100644 index 0000000000000..adbcedd91f205 --- /dev/null +++ b/src/test/compile-fail/mod_file_not_owning.rs @@ -0,0 +1,15 @@ +// Copyright 2014 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. + +// error-pattern: cannot declare a new module at this location + +mod mod_file_not_owning_aux1; + +fn main() {} diff --git a/src/test/compile-fail/mod_file_not_owning_aux1.rs b/src/test/compile-fail/mod_file_not_owning_aux1.rs new file mode 100644 index 0000000000000..2d522be6dc5dc --- /dev/null +++ b/src/test/compile-fail/mod_file_not_owning_aux1.rs @@ -0,0 +1,13 @@ +// Copyright 2014 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-test this is not a test + +mod mod_file_not_owning_aux2; diff --git a/src/test/compile-fail/mod_file_not_owning_aux2.rs b/src/test/compile-fail/mod_file_not_owning_aux2.rs new file mode 100644 index 0000000000000..41401d640f624 --- /dev/null +++ b/src/test/compile-fail/mod_file_not_owning_aux2.rs @@ -0,0 +1,11 @@ +// Copyright 2014 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-test this is not a test diff --git a/src/test/run-pass/mod_dir_simple/load_another_mod.rs b/src/test/run-pass/mod_dir_simple/load_another_mod.rs index c11b1e8c07469..ca45864a5a154 100644 --- a/src/test/run-pass/mod_dir_simple/load_another_mod.rs +++ b/src/test/run-pass/mod_dir_simple/load_another_mod.rs @@ -8,4 +8,5 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +#[path = "test.rs"] pub mod test;