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

Buildcheck - .editorconfig seems to be ignored #10684

Closed
JanKrivanek opened this issue Sep 22, 2024 · 6 comments
Closed

Buildcheck - .editorconfig seems to be ignored #10684

JanKrivanek opened this issue Sep 22, 2024 · 6 comments
Assignees
Labels
Area: BuildCheck Priority:1 Work that is critical for the release, but we could probably ship without triaged

Comments

@JanKrivanek
Copy link
Member

Context

I came accross a case where .editorconfig settings for chaning checks diagnostics levels are not respected and .editorconfig file is not captured in the binlog - meaning it's probably not recognized at all.

Details

Notice the BC0201 being set to error, but output as warning. Similarly BC0203 is set to error, but not lifted from default message.

PS C:\tmp> dotnet --version
9.0.100-rc.2.24468.2
PS C:\tmp> type .\.editorconfig
root = true

# Buildcheck rules
[*.csproj]
build_check.BC0103.severity=error
build_check.BC0203.severity=error
build_check.BC0203.scope=work_tree_imports
build_check.BC0201.severity=error
PS C:\tmp> dotnet build /check /bl
Obnovení dokončeno (1,4s)
Používáte verzi Preview rozhraní .NET. Viz: https://aka.ms/dotnet-support-policy
  DotUtils.Calculator akce proběhla úspěšně s 1 upozorněním(i). (1,4s) → bin\Debug\net8.0\DotUtils.Calculator.dll
    C:\tmp\DotUtils.Calculator.csproj(8,4): warning BC0201: https://aka.ms/buildcheck/codes#BC0201 - Property: 'xyz' was accessed, but it was never initialized.

Sestavení akce proběhla úspěšně s 1 upozorněním(i). za 5,1s
PS C:\tmp>

I suspect non-US locale might have an impact here?
Relevant info from systeminfo:

OS Name:                   Microsoft Windows 10 Home
OS Version:                10.0.19045 N/A Build 19045
System Locale:             cs;Čeština
Input Locale:              cs;Čeština

Binlog and repro:
ReproAndBinlog.zip

@JanKrivanek JanKrivanek added Area: BuildCheck Priority:1 Work that is critical for the release, but we could probably ship without labels Sep 22, 2024
@f-alizada
Copy link
Contributor

It is related to the selected end of line selected for the file.

var lines = string.IsNullOrEmpty(text) ? Array.Empty<string>() : text.Split(new string[] { Environment.NewLine }, StringSplitOptions.None);

We parse the string based on the current newLine of the OS.
Since the EOF was not the one that is used on current OS if failed to parse the file.

@f-alizada
Copy link
Contributor

f-alizada commented Sep 23, 2024

I see two possible ways:

  1. Identitfy the new line sequene of the file or
  2. Try to use the ReadLine from the StreamReader (if it supports this case)

@JanKrivanek
Copy link
Member Author

Thanks @f-alizada for the quick look!

What is the behavior of Roslyn (or possibly VS IDE settings)? We'd idealy be inline with that

@f-alizada
Copy link
Contributor

Roslyn covers the scenarios of different EOL by the SourceText , which was dropped during the implementation in MSBuild not to take the dependency.
The ReadLine handles the scenario of different EOL test on LF and CLRF which could be used.

@JanKrivanek
Copy link
Member Author

Let's do that.

The scenario of checking out the repo with specific EOL convention on a system with different convention is far too common

@JanProvaznik
Copy link
Contributor

fixed by #10740 and merged to main from vs17.12 branch in #10749

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: BuildCheck Priority:1 Work that is critical for the release, but we could probably ship without triaged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants