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

ESLint style single line ignore and lightbulb improvements #60668

Closed
johnnyreilly opened this issue Apr 9, 2022 · 8 comments
Closed

ESLint style single line ignore and lightbulb improvements #60668

johnnyreilly opened this issue Apr 9, 2022 · 8 comments

Comments

@johnnyreilly
Copy link

johnnyreilly commented Apr 9, 2022

The actual issue that I had in #60620 I now consider solved. (A better way of expressing it might be to say that it wasn't an issue; I just didn't understand correctly 😅)

I've got some developer experience feedback that has occurred to me, as a consequence of writing a blog post that talks about linting in C# as compared to JS/TS with ESLint. Essentially things that the JS/TS ecosystem has that are quite nice that dotnet does not.

I love the lightbulb 💡. When I'm developing, I love the auto fixes and suggestions that it provides.

I don't want to act upon each lint diagnostic. I may just want to ignore a particular one. This is made very easy in the JavaScript ecosystem for two reasons:

  1. It's a one liner with ESLint - I can deactivate linting for a single line with // eslint-disable-next-line.

screenshot of "// eslint-disable-next-line no-var"

  1. ESLint has a VS Code extension which for lints it will present a lightbulb 💡. And one of the options is "ignore the line" which will apply a // eslint-disable-next-line to do just that.

screenshot of the lightbulb menu in TS/JS including "Disable no-var for this line"

It would be awesome if a similar mechanisms existed in dotnet.

Single line ignore

Obviously I can ignore single lines in C#, but it takes me 2 lines of code to achieve it; like so:

#pragma warning disable IDE0052
    private readonly ILogger<WeatherForecastController> _logger;
#pragma warning restore IDE0052

What if instead, something like this existed:

#pragma warning disable-line IDE0052
    private readonly ILogger<WeatherForecastController> _logger;

Which disabled a single line of linting

Lightbulb

Imagine if the lightbulb with the C# extension offered the option to ignore a lint; just as the ESLint menu does. Quite the timesaver.

I'm not too sure where this feedback should go, but I thought I'd share it here to start with. What do people think? And more than that, is this the right place to discuss this?

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged Issues and PRs which have not yet been triaged by a lead label Apr 9, 2022
@Youssef1313
Copy link
Member

This is a language change. Not something Roslyn can do directly, I think. It should start as a discussion in dotnet/csharplang

@jinujoseph jinujoseph removed the untriaged Issues and PRs which have not yet been triaged by a lead label Apr 19, 2022
@jinujoseph jinujoseph added this to the Backlog milestone Apr 19, 2022
@jinujoseph
Copy link
Contributor

@sharwell

@sharwell
Copy link
Member

Duplicate of dotnet/csharplang#1070

@sharwell
Copy link
Member

Note that the light bulb already provides the option to suppress a diagnostic:
image

@johnnyreilly
Copy link
Author

johnnyreilly commented Apr 20, 2022

Hi @sharwell - I don't see that option in the lightbulb for VS Code - I think that may be a visual studio only option.

@johnnyreilly
Copy link
Author

Screenshot from 2022-04-20 06-14-56

This is what we see in VS Code.

@CyrusNajmabadi
Copy link
Member

@333fred for feedback on vscode.

@Youssef1313
Copy link
Member

@CyrusNajmabadi

[ExportConfigurationFixProvider(PredefinedConfigurationFixProviderNames.Suppression, LanguageNames.CSharp), Shared]
internal class CSharpSuppressionCodeFixProvider : AbstractSuppressionCodeFixProvider

This is the only codefix that doesn't have ExportCodeFixProvider and doesn't inherit CodeFixProvider (AbstractSuppressionCodeFixProvider only implements an IConfigurationFixProvider interface).

It seems there is a wrapper CodeFixProvider:

public WrapperCodeFixProvider(IConfigurationFixProvider suppressionFixProvider, IEnumerable<string> diagnosticIds)

But it also doesn't have the attribute (and its constructor is not parameterless).

Things around this CodeFixProvider is special cased here:

/// <summary> Looks explicitly for an <see cref="AbstractSuppressionCodeFixProvider"/>.</summary>
public CodeFixProvider? GetSuppressionFixer(string language, IEnumerable<string> diagnosticIds)
{
if (!_configurationProvidersMap.TryGetValue(language, out var lazyConfigurationProviders) ||
lazyConfigurationProviders.Value.IsDefault)
{
return null;
}
// Explicitly looks for an AbstractSuppressionCodeFixProvider
var fixer = lazyConfigurationProviders.Value.OfType<AbstractSuppressionCodeFixProvider>().FirstOrDefault();
if (fixer == null)
{
return null;
}
return new WrapperCodeFixProvider(fixer, diagnosticIds);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants