Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reexport tests without polluting namespaces #52890

Merged
merged 6 commits into from Aug 2, 2018
Merged
Diff settings

Always

Just for now

Copy path View file
@@ -15,6 +15,7 @@ use codemap::{ExpnInfo, MacroBang, MacroAttribute, dummy_spanned, respan};
use config::{is_test_or_bench, StripUnconfigured};
use errors::{Applicability, FatalError};
use ext::base::*;
use ext::build::AstBuilder;
use ext::derive::{add_derived_markers, collect_derives};
use ext::hygiene::{self, Mark, SyntaxContext};
use ext::placeholders::{placeholder, PlaceholderExpander};
@@ -474,6 +475,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
cx: self.cx,
invocations: Vec::new(),
monotonic: self.monotonic,
tests_nameable: true,
};
(fragment.fold_with(&mut collector), collector.invocations)
};
@@ -1049,6 +1051,11 @@ struct InvocationCollector<'a, 'b: 'a> {
cfg: StripUnconfigured<'a>,
invocations: Vec<Invocation>,
monotonic: bool,

/// Test functions need to be nameable. Tests inside functions or in other
/// unnameable locations need to be ignored. `tests_nameable` tracks whether
/// any test functions found in the current context would be nameable.
tests_nameable: bool,
}

impl<'a, 'b> InvocationCollector<'a, 'b> {
@@ -1066,6 +1073,20 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
placeholder(fragment_kind, NodeId::placeholder_from_mark(mark))
}

/// Folds the item allowing tests to be expanded because they are still nameable.
/// This should probably only be called with module items
fn fold_nameable(&mut self, item: P<ast::Item>) -> SmallVector<P<ast::Item>> {
fold::noop_fold_item(item, self)
}

/// Folds the item but doesn't allow tests to occur within it
fn fold_unnameable(&mut self, item: P<ast::Item>) -> SmallVector<P<ast::Item>> {
let was_nameable = mem::replace(&mut self.tests_nameable, false);
let items = fold::noop_fold_item(item, self);
self.tests_nameable = was_nameable;
items
}

fn collect_bang(&mut self, mac: ast::Mac, span: Span, kind: AstFragmentKind) -> AstFragment {
self.collect(kind, InvocationKind::Bang { mac: mac, ident: None, span: span })
}
@@ -1306,7 +1327,7 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> {
}
ast::ItemKind::Mod(ast::Mod { inner, .. }) => {
if item.ident == keywords::Invalid.ident() {
return noop_fold_item(item, self);
return self.fold_nameable(item);
}

let orig_directory_ownership = self.cx.current_expansion.directory_ownership;
@@ -1346,22 +1367,39 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> {

let orig_module =
mem::replace(&mut self.cx.current_expansion.module, Rc::new(module));
let result = noop_fold_item(item, self);
let result = self.fold_nameable(item);
self.cx.current_expansion.module = orig_module;
self.cx.current_expansion.directory_ownership = orig_directory_ownership;
result
}
// Ensure that test functions are accessible from the test harness.
ast::ItemKind::Fn(..) if self.cx.ecfg.should_test => {
if item.attrs.iter().any(|attr| is_test_or_bench(attr)) {
if self.tests_nameable && item.attrs.iter().any(|attr| is_test_or_bench(attr)) {
let orig_vis = item.vis.clone();

// Publicize the item under gensymed name to avoid pollution

This comment has been minimized.

Copy link
@petrochenkov

petrochenkov Jul 31, 2018

Contributor

Could you add a comment showing the transformation in code, like in #52890 (comment)?

item = item.map(|mut item| {
item.vis = respan(item.vis.span, ast::VisibilityKind::Public);
item.ident = Ident::from_interned_str(
item.ident.as_interned_str()).gensym();

This comment has been minimized.

Copy link
@petrochenkov

petrochenkov Jul 31, 2018

Contributor
orig_ident = item.ident;
item.ident = item.ident.gensym();
item
});

// Use the gensymed name under the item's original visibility
let use_item = self.cx.item_use_simple_(
item.ident.span,
orig_vis,
Some(Ident::from_interned_str(item.ident.as_interned_str())),

This comment has been minimized.

Copy link
@petrochenkov

petrochenkov Jul 31, 2018

Contributor

Some(orig_ident)

self.cx.path(item.ident.span, vec![Ident::from_str("self"), item.ident]));

This comment has been minimized.

Copy link
@petrochenkov

petrochenkov Jul 31, 2018

Contributor

keywords::SelfValue.ident()


SmallVector::many(
self.fold_unnameable(item).into_iter()
.chain(self.fold_unnameable(use_item)))
} else {
self.fold_unnameable(item)
}
noop_fold_item(item, self)
}
_ => noop_fold_item(item, self),
_ => self.fold_unnameable(item),
}
}

@@ -0,0 +1,38 @@
// Copyright 2015 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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// This test checks for namespace pollution by private tests.
// Tests used to marked as public causing name conflicts with normal
// functions only in test builds.

// compile-flags: --test

mod a {
pub fn foo() -> bool {
true
}
}

mod b {
#[test]
fn foo() {
local_name(); // ensure the local name still works
}

#[test]
fn local_name() {}
}

use a::*;
use b::*;

pub fn conflict() {
let _: bool = foo();
}
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.