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

Add code cleanup rules (+perf) #44269

Merged
merged 2 commits into from Oct 13, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
32 changes: 31 additions & 1 deletion .editorconfig
Expand Up @@ -229,13 +229,28 @@ dotnet_diagnostic.IDE0005.severity = warning
# IDE0011: Curly braces to surround blocks of code
dotnet_diagnostic.IDE0011.severity = warning

# IDE0020: Use pattern matching to avoid is check followed by a cast (with variable)
dotnet_diagnostic.IDE0020.severity = warning

# IDE0029: Use coalesce expression (non-nullable types)
dotnet_diagnostic.IDE0029.severity = warning

# IDE0030: Use coalesce expression (nullable types)
dotnet_diagnostic.IDE0030.severity = warning

# IDE0031: Use null propagation
dotnet_diagnostic.IDE0031.severity = warning

# IDE0035: Remove unreachable code
dotnet_diagnostic.IDE0035.severity = warning

# IDE0036: Order modifiers
csharp_preferred_modifier_order = public,private,protected,internal,static,extern,new,virtual,abstract,sealed,override,readonly,unsafe,volatile,async:suggestion
dotnet_diagnostic.IDE0036.severity = warning

# IDE0038: Use pattern matching to avoid is check followed by a cast (without variable)
dotnet_diagnostic.IDE0038.severity = warning

# IDE0043: Format string contains invalid placeholder
dotnet_diagnostic.IDE0043.severity = warning

Expand Down Expand Up @@ -265,11 +280,14 @@ file_header_template = Licensed to the .NET Foundation under one or more agreeme
# IDE0161: Convert to file-scoped namespace
dotnet_diagnostic.IDE0161.severity = warning

# IDE0200: Lambda expression can be removed
dotnet_diagnostic.IDE0200.severity = warning

# IDE2000: Disallow multiple blank lines
dotnet_style_allow_multiple_blank_lines_experimental = false
dotnet_diagnostic.IDE2000.severity = warning

[{eng/tools/**.cs,**/{test,testassets,samples,Samples,perf,scripts}/**.cs}]
[{eng/tools/**.cs,**/{test,testassets,samples,Samples,perf,scripts,stress}/**.cs}]
# CA1018: Mark attributes with AttributeUsageAttribute
dotnet_diagnostic.CA1018.severity = suggestion
# CA1507: Use nameof to express symbol names
Expand Down Expand Up @@ -320,6 +338,16 @@ dotnet_diagnostic.CA2012.severity = suggestion
dotnet_diagnostic.CA2249.severity = suggestion
# IDE0005: Remove unnecessary usings
dotnet_diagnostic.IDE0005.severity = suggestion
# IDE0020: Use pattern matching to avoid is check followed by a cast (with variable)
dotnet_diagnostic.IDE0020.severity = suggestion
# IDE0029: Use coalesce expression (non-nullable types)
dotnet_diagnostic.IDE0029.severity = suggestion
# IDE0030: Use coalesce expression (nullable types)
dotnet_diagnostic.IDE0030.severity = suggestion
# IDE0031: Use null propagation
dotnet_diagnostic.IDE0031.severity = suggestion
# IDE0038: Use pattern matching to avoid is check followed by a cast (without variable)
dotnet_diagnostic.IDE0038.severity = suggestion
# IDE0044: Make field readonly
dotnet_diagnostic.IDE0044.severity = suggestion
# IDE0051: Remove unused private members
Expand All @@ -330,6 +358,8 @@ dotnet_diagnostic.IDE0059.severity = suggestion
dotnet_diagnostic.IDE0060.severity = suggestion
# IDE0062: Make local function static
dotnet_diagnostic.IDE0062.severity = suggestion
# IDE0200: Lambda expression can be removed
dotnet_diagnostic.IDE0200.severity = suggestion

# CA2016: Forward the 'CancellationToken' parameter to methods that take one
dotnet_diagnostic.CA2016.severity = suggestion
Expand Down
2 changes: 1 addition & 1 deletion src/Components/Authorization/src/AuthorizeRouteView.cs
Expand Up @@ -38,7 +38,7 @@ public AuthorizeRouteView()
// Cache the rendering delegates so that we only construct new closure instances
// when they are actually used (e.g., we never prepare a RenderFragment bound to
// the NotAuthorized content except when you are displaying that particular state)
RenderFragment renderBaseRouteViewDelegate = builder => base.Render(builder);
RenderFragment renderBaseRouteViewDelegate = base.Render;
_renderAuthorizedDelegate = authenticateState => renderBaseRouteViewDelegate;
_renderNotAuthorizedDelegate = authenticationState => builder => RenderNotAuthorizedInDefaultLayout(builder, authenticationState);
_renderAuthorizingDelegate = RenderAuthorizingInDefaultLayout;
Expand Down
2 changes: 1 addition & 1 deletion src/Components/Server/src/Circuits/CircuitHost.cs
Expand Up @@ -586,7 +586,7 @@ public void SendPendingBatches()
// Dispatch any buffered renders we accumulated during a disconnect.
// Note that while the rendering is async, we cannot await it here. The Task returned by ProcessBufferedRenderBatches relies on
// OnRenderCompletedAsync to be invoked to complete, and SignalR does not allow concurrent hub method invocations.
_ = Renderer.Dispatcher.InvokeAsync(() => Renderer.ProcessBufferedRenderBatches());
_ = Renderer.Dispatcher.InvokeAsync(Renderer.ProcessBufferedRenderBatches);
}

private void AssertInitialized()
Expand Down
2 changes: 1 addition & 1 deletion src/Components/Server/src/Circuits/RemoteRenderer.cs
Expand Up @@ -175,7 +175,7 @@ public Task ProcessBufferedRenderBatches()
// All the batches are sent in order based on the fact that SignalR
// provides ordering for the underlying messages and that the batches
// are always in order.
return Task.WhenAll(_unacknowledgedRenderBatches.Select(b => WriteBatchBytesAsync(b)));
return Task.WhenAll(_unacknowledgedRenderBatches.Select(WriteBatchBytesAsync));
}

private async Task WriteBatchBytesAsync(UnacknowledgedRenderBatch pending)
Expand Down
Expand Up @@ -29,7 +29,7 @@ static JSComponentInterop()
{
if (HotReloadManager.Default.MetadataUpdateSupported)
{
HotReloadManager.Default.OnDeltaApplied += () => ParameterTypeCaches.Clear();
HotReloadManager.Default.OnDeltaApplied += ParameterTypeCaches.Clear;
}
}

Expand Down
Expand Up @@ -158,7 +158,7 @@ public IConfigurationBuilder Add(IConfigurationSource source)
// provider has reloaded data. This will invoke the RaiseChanged
// method which maps changes in individual providers to the change
// token on the WebAssemblyHostConfiguration object.
_changeTokenRegistrations.Add(ChangeToken.OnChange(() => provider.GetReloadToken(), () => RaiseChanged()));
_changeTokenRegistrations.Add(ChangeToken.OnChange(provider.GetReloadToken, RaiseChanged));

// We keep a list of providers in this class so that we can map
// set and get methods on this class to the set and get methods
Expand Down
Expand Up @@ -26,7 +26,10 @@ public static class WebAssemblyHotReload

internal static async Task InitializeAsync()
{
// Analyzer has a bug where it doesn't handle ConditionalAttribute: https://github.com/dotnet/roslyn/issues/63464
#pragma warning disable IDE0200 // Remove unnecessary lambda expression
_hotReloadAgent = new HotReloadAgent(m => Debug.WriteLine(m));
#pragma warning restore IDE0200 // Remove unnecessary lambda expression

if (Environment.GetEnvironmentVariable("__ASPNETCORE_BROWSER_TOOLS") == "true")
{
Expand Down
6 changes: 3 additions & 3 deletions src/FileProviders/Manifest.MSBuildTask/src/Manifest.cs
@@ -1,4 +1,4 @@
// Licensed to the .NET Foundation under one or more agreements.
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.IO;
Expand Down Expand Up @@ -46,7 +46,7 @@ public XDocument ToXmlDocument()
var root = new XElement(ElementNames.Root,
new XElement(ElementNames.ManifestVersion, "1.0"),
new XElement(ElementNames.FileSystem,
Root.Children.Select(e => BuildNode(e))));
Root.Children.Select(BuildNode)));

document.Add(root);

Expand All @@ -64,7 +64,7 @@ private XElement BuildNode(Entry entry)
else
{
var directory = new XElement(ElementNames.Directory, new XAttribute(ElementNames.Name, entry.Name));
directory.Add(entry.Children.Select(c => BuildNode(c)));
directory.Add(entry.Children.Select(BuildNode));
return directory;
}
}
Expand Down
Expand Up @@ -19,7 +19,7 @@ public class RemoteWindowsDeployer : ApplicationDeployer
private string _deployedFolderPathInFileShare;
private readonly RemoteWindowsDeploymentParameters _deploymentParameters;
private bool _isDisposed;
private static readonly Lazy<Scripts> _scripts = new Lazy<Scripts>(() => CopyEmbeddedScriptFilesToDisk());
private static readonly Lazy<Scripts> _scripts = new Lazy<Scripts>(CopyEmbeddedScriptFilesToDisk);

public RemoteWindowsDeployer(RemoteWindowsDeploymentParameters deploymentParameters, ILoggerFactory loggerFactory)
: base(deploymentParameters, loggerFactory)
Expand Down
4 changes: 2 additions & 2 deletions src/Http/Http.Abstractions/src/Internal/ParsingHelpers.cs
Expand Up @@ -63,7 +63,7 @@ public static void SetHeaderJoined(IHeaderDictionary headers, string key, String
}
else
{
headers[key] = string.Join(",", value.Select((s) => QuoteIfNeeded(s)));
headers[key] = string.Join(",", value.Select(QuoteIfNeeded));
}
}

Expand Down Expand Up @@ -135,7 +135,7 @@ public static void AppendHeaderJoined(IHeaderDictionary headers, string key, par
}
else
{
headers[key] = existing + "," + string.Join(",", values.Select(value => QuoteIfNeeded(value)));
headers[key] = existing + "," + string.Join(",", values.Select(QuoteIfNeeded));
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/Http/Routing/src/EndpointNameAddressScheme.cs
Expand Up @@ -70,7 +70,7 @@ public IEnumerable<Endpoint> FindEndpoints(string address)

// OK we need to report some duplicates.
var duplicates = endpoints
.GroupBy(e => GetEndpointName(e))
.GroupBy(GetEndpointName)
.Where(g => g.Key != null && g.Count() > 1);

var builder = new StringBuilder();
Expand Down
5 changes: 1 addition & 4 deletions src/Http/Routing/src/Matching/DfaMatcherFactory.cs
Expand Up @@ -32,9 +32,6 @@ public override Matcher CreateMatcher(EndpointDataSource dataSource)
// when the services are disposed.
var lifetime = _services.GetRequiredService<DataSourceDependentMatcher.Lifetime>();

return new DataSourceDependentMatcher(dataSource, lifetime, () =>
{
return _services.GetRequiredService<DfaMatcherBuilder>();
});
return new DataSourceDependentMatcher(dataSource, lifetime, _services.GetRequiredService<DfaMatcherBuilder>);
Copy link
Member

Choose a reason for hiding this comment

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

Is this right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, it's _services.GetRequiredService<DfaMatcherBuilder> not _services.GetRequiredService<DfaMatcherBuilder>(). (note the missing parens)

Copy link
Member

Choose a reason for hiding this comment

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

ahhhh

}
}
4 changes: 2 additions & 2 deletions src/Http/Routing/src/Matching/HostMatcherPolicy.cs
Expand Up @@ -210,7 +210,7 @@ public IReadOnlyList<PolicyNodeEdge> GetEdges(IReadOnlyList<Endpoint> endpoints)
for (var i = 0; i < endpoints.Count; i++)
{
var endpoint = endpoints[i];
var hosts = endpoint.Metadata.GetMetadata<IHostMetadata>()?.Hosts.Select(h => CreateEdgeKey(h)).ToArray();
var hosts = endpoint.Metadata.GetMetadata<IHostMetadata>()?.Hosts.Select(CreateEdgeKey).ToArray();
if (hosts == null || hosts.Length == 0)
{
hosts = new[] { EdgeKey.WildcardEdgeKey };
Expand All @@ -232,7 +232,7 @@ public IReadOnlyList<PolicyNodeEdge> GetEdges(IReadOnlyList<Endpoint> endpoints)
{
var endpoint = endpoints[i];

var endpointKeys = endpoint.Metadata.GetMetadata<IHostMetadata>()?.Hosts.Select(h => CreateEdgeKey(h)).ToArray() ?? Array.Empty<EdgeKey>();
var endpointKeys = endpoint.Metadata.GetMetadata<IHostMetadata>()?.Hosts.Select(CreateEdgeKey).ToArray() ?? Array.Empty<EdgeKey>();
if (endpointKeys.Length == 0)
{
// OK this means that this endpoint matches *all* hosts.
Expand Down
5 changes: 1 addition & 4 deletions src/Http/Routing/src/Matching/ILEmitTrieJumpTable.cs
Expand Up @@ -68,10 +68,7 @@ private int FallbackGetDestination(string path, PathSegment segment)
internal async Task InitializeILDelegateAsync()
{
// Offload the creation of the IL delegate to the thread pool.
await Task.Run(() =>
{
InitializeILDelegate();
});
await Task.Run(InitializeILDelegate);
}

// Internal for testing
Expand Down
2 changes: 1 addition & 1 deletion src/Identity/UI/src/IdentityDefaultUIConfigureOptions.cs
Expand Up @@ -34,7 +34,7 @@ public void PostConfigure(string? name, RazorPagesOptions options)
options.Conventions.AddAreaFolderApplicationModelConvention(
IdentityUIDefaultAreaName,
"/",
pam => convention.Apply(pam));
convention.Apply);
options.Conventions.AddAreaFolderApplicationModelConvention(
IdentityUIDefaultAreaName,
"/Account/Manage",
Expand Down
Expand Up @@ -88,10 +88,7 @@ public void Dispose()
{
Disposed = true;

if (_jsRuntime != null)
{
_jsRuntime.ReleaseObjectReference(_objectId);
}
_jsRuntime?.ReleaseObjectReference(_objectId);
}
}

Expand Down
Expand Up @@ -504,10 +504,7 @@ private static void Rethrow(ActionExecutedContextSealed? context)
return;
}

if (context.ExceptionDispatchInfo != null)
{
context.ExceptionDispatchInfo.Throw();
}
context.ExceptionDispatchInfo?.Throw();

if (context.Exception != null)
{
Expand Down
15 changes: 3 additions & 12 deletions src/Mvc/Mvc.Core/src/Infrastructure/ResourceInvoker.cs
Expand Up @@ -1447,10 +1447,7 @@ private static void Rethrow(ResourceExecutedContextSealed context)
return;
}

if (context.ExceptionDispatchInfo != null)
{
context.ExceptionDispatchInfo.Throw();
}
context.ExceptionDispatchInfo?.Throw();

if (context.Exception != null)
{
Expand All @@ -1470,10 +1467,7 @@ private static void Rethrow(ExceptionContextSealed context)
return;
}

if (context.ExceptionDispatchInfo != null)
{
context.ExceptionDispatchInfo.Throw();
}
context.ExceptionDispatchInfo?.Throw();

if (context.Exception != null)
{
Expand All @@ -1493,10 +1487,7 @@ private static void Rethrow(ResultExecutedContextSealed context)
return;
}

if (context.ExceptionDispatchInfo != null)
{
context.ExceptionDispatchInfo.Throw();
}
context.ExceptionDispatchInfo?.Throw();

if (context.Exception != null)
{
Expand Down
Expand Up @@ -79,7 +79,7 @@ protected void Subscribe()
if (_actions is ActionDescriptorCollectionProvider collectionProviderWithChangeToken)
{
_disposable = ChangeToken.OnChange(
() => collectionProviderWithChangeToken.GetChangeToken(),
collectionProviderWithChangeToken.GetChangeToken,
UpdateEndpoints);
}
}
Expand Down
Expand Up @@ -26,7 +26,7 @@ public void AddDataSource(ControllerActionEndpointDataSource dataSource)
public DynamicControllerEndpointSelector GetEndpointSelector(Endpoint endpoint)
{
var dataSourceId = endpoint.Metadata.GetMetadata<ControllerEndpointDataSourceIdMetadata>()!;
return _endpointSelectorCache.GetOrAdd(dataSourceId.Id, key => EnsureDataSource(key));
return _endpointSelectorCache.GetOrAdd(dataSourceId.Id, EnsureDataSource);
}

private DynamicControllerEndpointSelector EnsureDataSource(int key)
Expand Down
Expand Up @@ -124,7 +124,7 @@ public void CreateDisplayMetadata(DisplayMetadataProviderContext context)
}
else
{
displayMetadata.Description = () => displayAttribute.GetDescription();
displayMetadata.Description = displayAttribute.GetDescription;
}
}

Expand All @@ -146,7 +146,7 @@ public void CreateDisplayMetadata(DisplayMetadataProviderContext context)
}
else
{
displayMetadata.DisplayName = () => displayAttribute.GetName();
displayMetadata.DisplayName = displayAttribute.GetName;
}
}
else if (displayNameAttribute != null)
Expand Down Expand Up @@ -274,7 +274,7 @@ public void CreateDisplayMetadata(DisplayMetadataProviderContext context)
}
else
{
displayMetadata.Placeholder = () => displayAttribute.GetPrompt();
displayMetadata.Placeholder = displayAttribute.GetPrompt;
}
}

Expand Down
Expand Up @@ -11,6 +11,6 @@ internal sealed class RazorRuntimeCompilationHostingStartup : IHostingStartup
public void Configure(IWebHostBuilder builder)
{
// Add Razor services
builder.ConfigureServices(services => RazorRuntimeCompilationMvcCoreBuilderExtensions.AddServices(services));
builder.ConfigureServices(RazorRuntimeCompilationMvcCoreBuilderExtensions.AddServices);
}
}
2 changes: 1 addition & 1 deletion src/Mvc/Mvc.Razor/src/RazorPageActivator.cs
Expand Up @@ -39,7 +39,7 @@ public class RazorPageActivator : IRazorPageActivator

_propertyAccessors = new RazorPagePropertyActivator.PropertyValueAccessors
{
UrlHelperAccessor = context => urlHelperFactory.GetUrlHelper(context),
UrlHelperAccessor = urlHelperFactory.GetUrlHelper,
JsonHelperAccessor = context => jsonHelper,
DiagnosticSourceAccessor = context => diagnosticSource,
HtmlEncoderAccessor = context => htmlEncoder,
Expand Down
Expand Up @@ -31,7 +31,7 @@ internal sealed class DefaultPageFactoryProvider : IPageFactoryProvider
_modelMetadataProvider = metadataProvider;
_propertyAccessors = new RazorPagePropertyActivator.PropertyValueAccessors
{
UrlHelperAccessor = context => urlHelperFactory.GetUrlHelper(context),
UrlHelperAccessor = urlHelperFactory.GetUrlHelper,
JsonHelperAccessor = context => jsonHelper,
DiagnosticSourceAccessor = context => diagnosticListener,
HtmlEncoderAccessor = context => htmlEncoder,
Expand Down
Expand Up @@ -33,7 +33,7 @@ public void AddDataSource(PageActionEndpointDataSource dataSource)

var dataSourceId = endpoint.Metadata.GetMetadata<PageEndpointDataSourceIdMetadata>();
Debug.Assert(dataSourceId is not null);
return _endpointSelectorCache.GetOrAdd(dataSourceId.Id, key => EnsureDataSource(key));
return _endpointSelectorCache.GetOrAdd(dataSourceId.Id, EnsureDataSource);
}

private DynamicPageEndpointSelector EnsureDataSource(int key)
Expand Down
Expand Up @@ -703,10 +703,7 @@ private static void Rethrow(PageHandlerExecutedContext? context)
return;
}

if (context.ExceptionDispatchInfo != null)
{
context.ExceptionDispatchInfo.Throw();
}
context.ExceptionDispatchInfo?.Throw();

if (context.Exception != null)
{
Expand Down
Expand Up @@ -104,9 +104,6 @@ public virtual Task ExecuteAsync(PageContext pageContext, PageResult result)
private static void OnExecuting(PageContext pageContext)
{
var viewDataValuesProvider = pageContext.HttpContext.Features.Get<IViewDataValuesProviderFeature>();
if (viewDataValuesProvider != null)
{
viewDataValuesProvider.ProvideViewDataValues(pageContext.ViewData);
}
viewDataValuesProvider?.ProvideViewDataValues(pageContext.ViewData);
}
}