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

Fix bug in shebang handling #71487

Merged
merged 1 commit into from
May 26, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 19 additions & 7 deletions src/librustc_lexer/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,16 +236,28 @@ pub enum Base {
}

/// `rustc` allows files to have a shebang, e.g. "#!/usr/bin/rustrun",
/// but shebang isn't a part of rust syntax, so this function
/// skips the line if it starts with a shebang ("#!").
/// Line won't be skipped if it represents a valid Rust syntax
/// (e.g. "#![deny(missing_docs)]").
/// but shebang isn't a part of rust syntax.
pub fn strip_shebang(input: &str) -> Option<usize> {
debug_assert!(!input.is_empty());
if !input.starts_with("#!") || input.starts_with("#![") {
let first_line = input.lines().next()?;
// A shebang is intentionally loosely defined as `#! [non whitespace]` on the first line.
let could_be_shebang =
first_line.starts_with("#!") && first_line[2..].contains(|c| !is_whitespace(c));
if !could_be_shebang {
return None;
}
Some(input.find('\n').unwrap_or(input.len()))
let non_whitespace_tokens = tokenize(input).map(|tok| tok.kind).filter(|tok|
!matches!(tok, TokenKind::LineComment | TokenKind::BlockComment { .. } | TokenKind::Whitespace)
);
let prefix = [TokenKind::Pound, TokenKind::Not, TokenKind::OpenBracket];
let starts_with_attribute = non_whitespace_tokens.take(3).eq(prefix.iter().copied());
if starts_with_attribute {
// If the file starts with #![ then it's definitely not a shebang -- it couldn't be
// a rust program since a Rust program can't start with `[`
None
} else {
// It's a #!... and there isn't a `[` in sight, must be a shebang
Some(first_line.len())
}
}

/// Parses the first token from the provided input string.
Expand Down
57 changes: 57 additions & 0 deletions src/librustc_lexer/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,4 +145,61 @@ mod tests {
}),
);
}

#[test]
fn test_valid_shebang() {
// https://github.com/rust-lang/rust/issues/70528
let input = "#!/usr/bin/rustrun\nlet x = 5;";
assert_eq!(strip_shebang(input), Some(18));
}

#[test]
fn test_invalid_shebang_valid_rust_syntax() {
// https://github.com/rust-lang/rust/issues/70528
let input = "#! [bad_attribute]";
assert_eq!(strip_shebang(input), None);
}

#[test]
fn test_shebang_second_line() {
// Because shebangs are interpreted by the kernel, they must be on the first line
let input = "\n#!/bin/bash";
assert_eq!(strip_shebang(input), None);
}

#[test]
fn test_shebang_space() {
let input = "#! /bin/bash";
assert_eq!(strip_shebang(input), Some(input.len()));
}

#[test]
fn test_shebang_empty_shebang() {
let input = "#! \n[attribute(foo)]";
assert_eq!(strip_shebang(input), None);
}

#[test]
fn test_invalid_shebang_comment() {
let input = "#!//bin/ami/a/comment\n[";
assert_eq!(strip_shebang(input), None)
}

#[test]
fn test_invalid_shebang_another_comment() {
let input = "#!/*bin/ami/a/comment*/\n[attribute";
assert_eq!(strip_shebang(input), None)
}

#[test]
fn test_shebang_valid_rust_after() {
let input = "#!/*bin/ami/a/comment*/\npub fn main() {}";
assert_eq!(strip_shebang(input), Some(23))
}

#[test]
fn test_shebang_followed_by_attrib() {
let input = "#!/bin/rust-scripts\n#![allow_unused(true)]";
assert_eq!(strip_shebang(input), Some(19));
}
}
2 changes: 2 additions & 0 deletions src/test/ui/parser/shebang/issue-71471-ignore-tidy.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@

#!B //~ expected `[`, found `B`
8 changes: 8 additions & 0 deletions src/test/ui/parser/shebang/issue-71471-ignore-tidy.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
error: expected `[`, found `B`
--> $DIR/issue-71471-ignore-tidy.rs:2:3
|
LL | #!B
| ^ expected `[`

error: aborting due to previous error

7 changes: 7 additions & 0 deletions src/test/ui/parser/shebang/multiline-attrib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#!
[allow(unused_variables)]
// check-pass

fn main() {
let x = 5;
}
5 changes: 5 additions & 0 deletions src/test/ui/parser/shebang/regular-attrib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#![allow(unused_variables)]
// check-pass
fn main() {
let x = 5;
}
9 changes: 9 additions & 0 deletions src/test/ui/parser/shebang/shebang-and-attrib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#!/usr/bin/env run-cargo-script

// check-pass
#![allow(unused_variables)]


fn main() {
let x = 5;
}
6 changes: 6 additions & 0 deletions src/test/ui/parser/shebang/shebang-comment.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#!//bin/bash

// check-pass
fn main() {
println!("a valid shebang (that is also a rust comment)")
}
6 changes: 6 additions & 0 deletions src/test/ui/parser/shebang/shebang-must-start-file.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// something on the first line for tidy
#!/bin/bash //~ expected `[`, found `/`

fn main() {
println!("ok!");
}
8 changes: 8 additions & 0 deletions src/test/ui/parser/shebang/shebang-must-start-file.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
error: expected `[`, found `/`
--> $DIR/shebang-must-start-file.rs:2:3
|
LL | #!/bin/bash
| ^ expected `[`

error: aborting due to previous error

16 changes: 16 additions & 0 deletions src/test/ui/parser/shebang/sneaky-attrib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#!//bin/bash


// This could not possibly be a shebang & also a valid rust file, since a Rust file
// can't start with `[`
/*
[ (mixing comments to also test that we ignore both types of comments)

*/

[allow(unused_variables)]

// check-pass
fn main() {
let x = 5;
}
6 changes: 6 additions & 0 deletions src/test/ui/parser/shebang/valid-shebang.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#!/usr/bin/env run-cargo-script

// check-pass
fn main() {
println!("Hello World!");
}
5 changes: 5 additions & 0 deletions src/tools/tidy/src/style.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,11 @@ pub fn check(path: &Path, bad: &mut bool) {

let can_contain =
contents.contains("// ignore-tidy-") || contents.contains("# ignore-tidy-");
// Enable testing ICE's that require specific (untidy)
// file formats easily eg. `issue-1234-ignore-tidy.rs`
if filename.contains("ignore-tidy") {
return;
}
Comment on lines +177 to +181
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a hack in the best meaning of the word.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also place these kind of tests in a specific directory and filter those out?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The filename opt out looks good to me.
It seems better to keep tests in directories where they actually belong to.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let mut skip_cr = contains_ignore_directive(can_contain, &contents, "cr");
let mut skip_undocumented_unsafe =
contains_ignore_directive(can_contain, &contents, "undocumented-unsafe");
Expand Down