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

cargo (for cargo script) and rustc disagee on shebangs #15170

Closed
epage opened this issue Feb 11, 2025 · 2 comments · Fixed by #15173
Closed

cargo (for cargo script) and rustc disagee on shebangs #15170

epage opened this issue Feb 11, 2025 · 2 comments · Fixed by #15173
Labels
C-bug Category: bug S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. Z-script Nightly: cargo script

Comments

@epage
Copy link
Contributor

epage commented Feb 11, 2025

Rustc has logic to detect and ignore shebangs
https://github.com/rust-lang/rust/blob/6171d944aea415a3023d4262e0895aa3b18c771f/compiler/rustc_lexer/src/lib.rs#L255-L278

Cargo needs something similar for knowing whether we should look for a frontmatter

// See rust-lang/rust's compiler/rustc_lexer/src/lib.rs's `strip_shebang`
// Shebang must start with `#!` literally, without any preceding whitespace.
// For simplicity we consider any line starting with `#!` a shebang,
// regardless of restrictions put on shebangs by specific platforms.
if let Some(rest) = source.content.strip_prefix("#!") {
// Ok, this is a shebang but if the next non-whitespace token is `[`,
// then it may be valid Rust code, so consider it Rust code.
if rest.trim_start().starts_with('[') {
return Ok(source);
}
// No other choice than to consider this a shebang.
let newline_end = source
.content
.find('\n')
.map(|pos| pos + 1)
.unwrap_or(source.content.len());
let (shebang, content) = source.content.split_at(newline_end);
source.shebang = Some(shebang);
source.content = content;
}

These differ and we need to decide how we want to reconcile these

Originally reported in #14857 (comment)

@epage epage added C-bug Category: bug S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. Z-script Nightly: cargo script labels Feb 11, 2025
@epage
Copy link
Contributor Author

epage commented Feb 11, 2025

The key problem is #! can either start a shebang or an attribute and we need to be able to tell which we are seeing on the first line.

Ours is implemented to the textual description (#! + whitespace + not [) but misses that "whitespace" can refer to space, tab, newlines (I think), line comments (including doc comments), and block comments (including doc comments). As we use trim_start, we do handle space, tab, and newlines. We are only missing comments.

Part of our intention with the frontmatter syntax is we don't have to have a full Rust parser, so I worry about having to deal with some of this extra syntax.

@epage
Copy link
Contributor Author

epage commented Feb 11, 2025

An example of something that rustc considers to not have a shebang:

#!// Hello
[allow(dead_code)]

fn main() {
    println!("Hello, world!");
}

I lean towards

  • Keeping what we have, treating the above as having a shebang
  • Change logic to require one non-whitespace character between #! and \n
    • This works well for rustc allowing comments on following lines but not when rustc allows comments on the same line as #!
    • However, a user doing #! // or #! /* without it being a shebang is odd

epage added a commit to epage/cargo that referenced this issue Feb 11, 2025
rustc considers the following valid and without a shebang:
```rust

// Hello

[allow(dead_code)]

fn main() {
    println!("Hello, world!");
}
```
and
```rustc
[allow(dead_code)]

fn main() {
    println!("Hello, world!");
}
```

In both cases, we consider it to have a shebang.  This commit documents
that intention.

We could add our own heuristics
(e.g. `#!` with only whitespace is not a shebang)
but we should either be a subset or intentionally different than rustc
(e.g. require a non `[`-prefixes interpreter)
rather than do both.

This will be reflected in the tracking issue which will handle the final
decision for the team on this matter.

Fixes rust-lang#15170
github-merge-queue bot pushed a commit that referenced this issue Feb 11, 2025
### What does this PR try to resolve?

rustc considers the following valid and without a shebang:
```rust

// Hello

[allow(dead_code)]

fn main() {
    println!("Hello, world!");
}
```
and
```rustc
[allow(dead_code)]

fn main() {
    println!("Hello, world!");
}
```

In both cases, we consider it to have a shebang. This commit documents
that intention.

We could add our own heuristics
(e.g. `#!` with only whitespace is not a shebang)
but we should either be a subset or intentionally different than rustc
(e.g. require a non `[`-prefixes interpreter)
rather than do both.

Fixes #15170

### How should we test and review this PR?

This will be reflected in the tracking issue which will handle the final
decision for the team on this matter.

### Additional information
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. Z-script Nightly: cargo script
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant