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

Multi-line diagnostic suggestions use LF in files that use CRLF line endings #119482

Open
ehuss opened this issue Jan 1, 2024 · 2 comments
Open
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` C-bug Category: This is a bug. O-windows Operating system: Windows T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@ehuss
Copy link
Contributor

ehuss commented Jan 1, 2024

Diagnostic suggestions that include line endings only use LF line endings. However, if the file is using CRLF line endings, applying the suggestion results in a file with mixed line endings.

I would expect that suggestions should match the line-ending style of the file.

For example:

mod a {
    pub fn f() {}
}

fn main() {
    f();
}

generates JSON:

{
    "$message_type": "diagnostic",
    "message": "cannot find function `f` in this scope",
    "code":
    {
        "code": "E0425",
        "explanation": "An unresolved name was used.\n\nErroneous code examples:\n\n```compile_fail,E0425\nsomething_that_doesnt_exist::foo;\n// error: unresolved name `something_that_doesnt_exist::foo`\n\n// or:\n\ntrait Foo {\n    fn bar() {\n        Self; // error: unresolved name `Self`\n    }\n}\n\n// or:\n\nlet x = unknown_variable;  // error: unresolved name `unknown_variable`\n```\n\nPlease verify that the name wasn't misspelled and ensure that the\nidentifier being referred to is valid for the given situation. Example:\n\n```\nenum something_that_does_exist {\n    Foo,\n}\n```\n\nOr:\n\n```\nmod something_that_does_exist {\n    pub static foo : i32 = 0i32;\n}\n\nsomething_that_does_exist::foo; // ok!\n```\n\nOr:\n\n```\nlet unknown_variable = 12u32;\nlet x = unknown_variable; // ok!\n```\n\nIf the item is not defined in the current module, it must be imported using a\n`use` statement, like so:\n\n```\n# mod foo { pub fn bar() {} }\n# fn main() {\nuse foo::bar;\nbar();\n# }\n```\n\nIf the item you are importing is not defined in some super-module of the\ncurrent module, then it must also be declared as public (e.g., `pub fn`).\n"
    },
    "level": "error",
    "spans":
    [
        {
            "file_name": "main.rs",
            "byte_start": 50,
            "byte_end": 51,
            "line_start": 6,
            "line_end": 6,
            "column_start": 5,
            "column_end": 6,
            "is_primary": true,
            "text":
            [
                {
                    "text": "    f();",
                    "highlight_start": 5,
                    "highlight_end": 6
                }
            ],
            "label": "not found in this scope",
            "suggested_replacement": null,
            "suggestion_applicability": null,
            "expansion": null
        }
    ],
    "children":
    [
        {
            "message": "consider importing this function",
            "code": null,
            "level": "help",
            "spans":
            [
                {
                    "file_name": "main.rs",
                    "byte_start": 0,
                    "byte_end": 0,
                    "line_start": 1,
                    "line_end": 1,
                    "column_start": 1,
                    "column_end": 1,
                    "is_primary": true,
                    "text":
                    [],
                    "label": null,
                    "suggested_replacement": "use a::f;\n\n",
                    "suggestion_applicability": "MaybeIncorrect",
                    "expansion": null
                }
            ],
            "children":
            [],
            "rendered": null
        }
    ],
    "rendered": "error[E0425]: cannot find function `f` in this scope\n --> main.rs:6:5\n  |\n6 |     f();\n  |     ^ not found in this scope\n  |\nhelp: consider importing this function\n  |\n1 + use a::f;\n  |\n\n"
}

Notice that the suggested_replacement uses \n despite the source file using \r\n line endings.

Meta

rustc --version --verbose:

rustc 1.77.0-nightly (3cdd004e5 2023-12-29)
binary: rustc
commit-hash: [3cdd004e55c869faa2b7b25efd3becf50346e7d6](https://github.com/rust-lang/rust/commit/3cdd004e55c869faa2b7b25efd3becf50346e7d6)
commit-date: 2023-12-29
host: x86_64-pc-windows-msvc
release: 1.77.0-nightly
LLVM version: 17.0.6
@ehuss ehuss added O-windows Operating system: Windows C-bug Category: This is a bug. A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` labels Jan 1, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jan 1, 2024
ehuss added a commit to ehuss/cargo that referenced this issue Jan 1, 2024
Suggestions that come from rustc that are multi-line only use LF line
endings. But if the file is checked out on windows with CRLF
line-endings, then you end up with a mix of line endings that don't
match the "fixed.rs" file.

Tracking this at rust-lang/rust#119482.
@Noratrieb Noratrieb added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-diagnostics Area: Messages for errors, warnings, and lints and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Jan 1, 2024
@bjorn3
Copy link
Member

bjorn3 commented Jan 1, 2024

All newlines are normalized to LF when reading sources files. This happens before even lexing them. We did have to somehow preserve the knowledge that CRLF was used.

@chenyukang
Copy link
Member

chenyukang commented Jan 12, 2024

yeah, this is tracked at #62865
which is the proper data structure to preserve the knowledge CRLF presented? maybe SourceFile if we assume a file only uses one of them?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` C-bug Category: This is a bug. O-windows Operating system: Windows T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants