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.

Sign up for GitHub

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

Jump to bottom

Enforce style rules during build #5796

Closed
wants to merge 5 commits into from
Closed

Enforce style rules during build #5796

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4c79e0b
enforce rules for a project
jgonz120 Apr 3, 2024
c8123b3
add change to csproj to break build
jgonz120 Apr 6, 2024
81600ae
Fix build errors
jgonz120 Apr 6, 2024
ab9b8ec
removed supressing from common targets and supressed it per file.
jgonz120 May 10, 2024
09d663d
revert editor config
jgonz120 May 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions build/Shared/EncodingUtility.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@

#nullable enable

using System;

namespace NuGet.Shared
{
internal static class EncodingUtility
Expand Down
1 change: 0 additions & 1 deletion build/Shared/SharedExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace NuGet.Shared
Expand Down
4 changes: 2 additions & 2 deletions build/Shared/TaskResultCache.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ internal sealed class TaskResultCache<TKey, TValue>
private readonly ConcurrentDictionary<TKey, object> _perTaskLock;

/// <summary>
/// Initializes a new instance of the <see cref="ResultCache{TKey, TValue}" /> class with the specified key comparer.
/// Initializes a new instance of the <see cref="TaskResultCache{TKey, TValue}" /> class with the specified key comparer.
/// </summary>
/// <param name="comparer">An <see cref="IEqualityComparer{T}" /> to use when comparing keys.</param>
public TaskResultCache(IEqualityComparer<TKey> comparer)
Expand All @@ -39,7 +39,7 @@ public TaskResultCache(IEqualityComparer<TKey> comparer)
}

/// <summary>
/// Initializes a new instance of the <see cref="ResultCache{TKey, TValue}" /> class.
/// Initializes a new instance of the <see cref="TaskResultCache{TKey, TValue}" /> class.
/// </summary>
public TaskResultCache()
{
Expand Down
2 changes: 1 addition & 1 deletion build/common.project.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@
</ItemGroup>

<ItemGroup Condition=" '$(IsXPlat)' != 'true' ">
<ApexProjects Include="$(RepositoryRootDirectory)test\NuGet.Tests.Apex\*\*.csproj" />
<ApexProjects Include="$(RepositoryRootDirectory)test\NuGet.Tests.Apex\*\*.csproj" />
</ItemGroup>

<ItemGroup Condition=" '$(IsXPlat)' != 'true' ">
Expand Down
17 changes: 9 additions & 8 deletions build/common.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
</PropertyGroup>

<ImportGroup Condition=" '$(PackProject)' == 'true' AND '$(NuGetBuildTasksPackTargets)' != '' ">
<Import Project="$(NuGetBuildTasksPackTargets)" />
<Import Project="$(NuGetBuildTasksPackTargets)" />
</ImportGroup>

<!-- Readme file in shipping packages -->
Expand All @@ -55,7 +55,7 @@
<None Include="$(MSBuildProjectDirectory)\README.md" Pack="true" PackagePath="\"/>
</ItemGroup>

<!-- Don't use PublicApiAnalyzer on source-build .NET -->
<!-- Don't use PublicApiAnalyzer on source-build .NET -->
<PropertyGroup Condition=" '$(DotNetBuildSourceOnly)' == 'true' ">
<UsePublicApiAnalyzer>false</UsePublicApiAnalyzer>
</PropertyGroup>
Expand All @@ -78,7 +78,7 @@
<ImportGroup Condition=" '$(TestProject)' == 'true' ">
<Import Project="test.targets" />
</ImportGroup>


<!-- Allow WPF projects to run under NETCore SDK -->
<!-- Errors occur if the output path is not set correctly -->
Expand Down Expand Up @@ -150,7 +150,7 @@
============================================================
-->
<Target Name="GetSymbolsToIndex" DependsOnTargets="$(GetSymbolsToIndexDependsOn)" Returns="@(SymbolFilesToIndex)" />

<Target Name="GetSymbolsToIndexDefault" DependsOnTargets="GetTargetFrameworkSet" Returns="@(SymbolFilesToIndex)" Condition=" '$(Shipping)' == 'true' ">
<MSBuild
Projects="$(MSBuildProjectFullPath)"
Expand All @@ -172,8 +172,8 @@
============================================================
-->
<Target Name="GetSigningInputs" DependsOnTargets="$(GetSigningInputsDependsOn)" Returns="@(DllsToSign)" />
<!--

<!--
============================================================
The default target to get signing inputs for a particular project
============================================================
Expand Down Expand Up @@ -258,7 +258,8 @@
<!-- remove all net45 assemblies, otherwise loc tools have problem loading two assemblies of same name in same appdomain. net45 doesn't ship in VSIX, so doesn't need loc. -->
<DllsToLocalizeWithMetadata Include="@(BuiltProjectOutputGroupOutput->'%(FinalOutputPath)')"
Condition=" ('%(Extension)' == '.dll' OR '%(Filename)' == 'NuGet.CommandLine.XPlat' OR '%(Filename)' == 'NuGet') AND !($([System.String]::new('%(FullPath)').Contains('\net45\'))) " KeepDuplicates="false">
<TranslationFile>$(LocalizationWorkDirectory)\{Lang}\15\%(Filename)%(Extension).lcl</TranslationFile> <!--Required: translation file-->
<TranslationFile>$(LocalizationWorkDirectory)\{Lang}\15\%(Filename)%(Extension).lcl</TranslationFile>
<!--Required: translation file-->
<LciCommentFile>$(LocalizationWorkDirectory)\comments\15\%(Filename)%(Extension).lci</LciCommentFile>
<HasLceComments>false</HasLceComments>
</DllsToLocalizeWithMetadata>
Expand Down Expand Up @@ -377,7 +378,7 @@ Condition=" ('%(Extension)' == '.dll' OR '%(Filename)' == 'NuGet.CommandLine.XPl
</Target>

<Target Name="GetAllTargetFrameworks">
<PropertyGroup>
<PropertyGroup>
<EffectiveFramework Condition=" '$(TargetFrameworks)' != '' ">$(TargetFrameworks)</EffectiveFramework>
<EffectiveFramework Condition=" '$(EffectiveFramework)' == '' ">$(TargetFramework)</EffectiveFramework>
</PropertyGroup>
Expand Down
6 changes: 6 additions & 0 deletions src/NuGet.Clients/NuGet.CommandLine/CommandLineMachineWideSettings.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,23 @@
namespace NuGet.CommandLine
{
[Export(typeof(Configuration.IMachineWideSettings))]
#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member
Copy link
Member

Choose a reason for hiding this comment

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

We've historically added our suppressions in a GlobalSuppressions file, since this makes the file really busy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It didn't give me the option to in the "Quick actions" even after creating a global suppression file. Lemme see just force the value in there and see if it works

Copy link
Member

Choose a reason for hiding this comment

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

I 100% disagree with putting all the suppressions in a GlobalSuppressions file. It being busy in the code is a feature, because when I'm reviewing a future PR and I see they're modifying a line within a #pragma disable block, or a line right next to it, I can comment on the PR saying please remove this tech debt. If the suppression is in the GlobalSuppresions file, it becomes much more difficult to know that there's an analyzer warning that's being suppressed and that improvements could be made.

If this is specifically about VS1591 or other XML comment warnings, then maybe we should disable this analyzer for proejcts that don't ship to nuget.org as packages.

I just don't see GlobalSuppressions.cs as a positive thing, with the possible exception of WPF code and the "keep the object heirarchy lower than X" warning.

public class CommandLineMachineWideSettings : Configuration.IMachineWideSettings
#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member
{
Lazy<Configuration.ISettings> _settings;

#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member
public CommandLineMachineWideSettings()
#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member
{
var baseDirectory = NuGetEnvironment.GetFolderPath(NuGetFolderPath.MachineWideConfigDirectory);
_settings = new Lazy<Configuration.ISettings>(
() => Configuration.Settings.LoadMachineWideSettings(baseDirectory));
}

#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member
public Configuration.ISettings Settings => _settings.Value;
#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member
}
}
11 changes: 10 additions & 1 deletion src/NuGet.Clients/NuGet.CommandLine/CommandLineParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using System.Linq;
using System.Reflection;
Expand All @@ -13,19 +12,25 @@

namespace NuGet.CommandLine
{
#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member
public class CommandLineParser
#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member
{
private readonly ICommandManager _commandManager;

// On Unix or MacOSX slash as a switch indicator would interfere with the path separator
private static readonly bool _supportSlashAsSwitch = (Environment.OSVersion.Platform != PlatformID.Unix) && (Environment.OSVersion.Platform != PlatformID.MacOSX);

#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member
public CommandLineParser(ICommandManager manager)
#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member
{
_commandManager = manager;
}

#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member
public void ExtractOptions(ICommand command, IEnumerator<string> argsEnumerator)
#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member
{
List<string> arguments = new List<string>();
IDictionary<OptionAttribute, PropertyInfo> properties = _commandManager.GetCommandOptions(command);
Expand Down Expand Up @@ -131,7 +136,9 @@ internal static void AssignValue(object command, PropertyInfo property, string o
}
}

#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member
public ICommand ParseCommandLine(IEnumerable<string> commandLineArgs)
#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member
{
IEnumerator<string> argsEnumerator = commandLineArgs.GetEnumerator();

Expand All @@ -153,7 +160,9 @@ public ICommand ParseCommandLine(IEnumerable<string> commandLineArgs)
return cmd;
}

#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member
public static string GetNextCommandLineItem(IEnumerator<string> argsEnumerator)
#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member
{
if (argsEnumerator == null || !argsEnumerator.MoveNext())
{
Expand Down
6 changes: 6 additions & 0 deletions src/NuGet.Clients/NuGet.CommandLine/CommandLineSourceRepositoryProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ public class CommandLineSourceRepositoryProvider : ISourceRepositoryProvider
private static readonly ConcurrentDictionary<Configuration.PackageSource, SourceRepository> _cachedSources
= new ConcurrentDictionary<Configuration.PackageSource, SourceRepository>();

#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member
public CommandLineSourceRepositoryProvider(Configuration.IPackageSourceProvider packageSourceProvider)
#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member
{
_packageSourceProvider = packageSourceProvider;

Expand Down Expand Up @@ -50,12 +52,16 @@ public SourceRepository CreateRepository(Configuration.PackageSource source)
return CreateRepository(source, FeedType.Undefined);
}

#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member
public SourceRepository CreateRepository(Configuration.PackageSource source, FeedType type)
#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member
{
return _cachedSources.GetOrAdd(source, new SourceRepository(source, _resourceProviders, type));
}

#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member
public Configuration.IPackageSourceProvider PackageSourceProvider
#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member
{
get { return _packageSourceProvider; }
}
Expand Down
10 changes: 10 additions & 0 deletions src/NuGet.Clients/NuGet.CommandLine/CommandManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,22 @@
namespace NuGet.CommandLine
{
[Export(typeof(ICommandManager))]
#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member
public class CommandManager : ICommandManager
#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member
{
private readonly IList<ICommand> _commands = new List<ICommand>();

#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member
public IEnumerable<ICommand> GetCommands()
#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member
{
return _commands;
}

#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member
public ICommand GetCommand(string commandName)
#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member
{
IEnumerable<ICommand> results = from command in _commands
where command.CommandAttribute.CommandName.StartsWith(commandName, StringComparison.OrdinalIgnoreCase) ||
Expand Down Expand Up @@ -50,7 +56,9 @@ public ICommand GetCommand(string commandName)
return matchedCommand;
}

#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member
public IDictionary<OptionAttribute, PropertyInfo> GetCommandOptions(ICommand command)
#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member
{
var result = new Dictionary<OptionAttribute, PropertyInfo>();

Expand All @@ -77,7 +85,9 @@ public ICommand GetCommand(string commandName)
return result;
}

#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member
public void RegisterCommand(ICommand command)
#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member
{
var attrib = command.CommandAttribute;
if (attrib != null)
Expand Down
8 changes: 8 additions & 0 deletions src/NuGet.Clients/NuGet.CommandLine/Commands/AddCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,23 @@ namespace NuGet.CommandLine
[Command(typeof(NuGetCommand), "add", "AddCommandDescription",
MinArgs = 1, MaxArgs = 1, UsageDescriptionResourceName = "AddCommandUsageDescription",
UsageSummaryResourceName = "AddCommandUsageSummary", UsageExampleResourceName = "AddCommandUsageExamples")]
#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member
public class AddCommand : Command
#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member
{
[Option(typeof(NuGetCommand), "AddCommandSourceDescription", AltName = "src")]
#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member
public string Source { get; set; }
#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member

[Option(typeof(NuGetCommand), "ExpandDescription")]
#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member
public bool Expand { get; set; }
#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member

#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member
public override async Task ExecuteCommandAsync()
#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member
{
// Arguments[0] will not be null at this point.
// Because, this command has MinArgs set to 1.
Expand Down
22 changes: 22 additions & 0 deletions src/NuGet.Clients/NuGet.CommandLine/Commands/ClientCertificatesCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,40 +15,62 @@ namespace NuGet.CommandLine.Commands
MinArgs = 0,
UsageSummaryResourceName = "ClientCertificatesCommandUsageSummary",
UsageExampleResourceName = "ClientCertificatesCommandUsageExamples")]
#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member
public class ClientCertificatesCommand : Command
#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member
{
internal ClientCertificatesCommand()
{
}

[Option(typeof(NuGetCommand), "ClientCertificatesCommandFindByDescription", AltName = nameof(IClientCertArgsWithStoreData.FindBy))]
#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member
public string FindBy { get; set; }
#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member

[Option(typeof(NuGetCommand), "ClientCertificatesCommandFindValueDescription", AltName = nameof(IClientCertArgsWithStoreData.FindValue))]
#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member
public string FindValue { get; set; }
#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member

[Option(typeof(NuGetCommand), "ClientCertificatesCommandForceDescription", AltName = nameof(IClientCertArgsWithForce.Force))]
#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member
public bool Force { get; set; }
#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member

[Option(typeof(NuGetCommand), "ClientCertificatesCommandPackageSourceDescription", AltName = nameof(IClientCertArgsWithPackageSource.PackageSource))]
#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member
public string PackageSource { get; set; }
#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member

[Option(typeof(NuGetCommand), "ClientCertificatesCommandPasswordDescription", AltName = nameof(IClientCertArgsWithFileData.Password))]
#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member
public string Password { get; set; }
#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member

[Option(typeof(NuGetCommand), "ClientCertificatesCommandFilePathDescription", AltName = nameof(IClientCertArgsWithFileData.Path))]
#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member
public string Path { get; set; }
#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member

[Option(typeof(NuGetCommand), "ClientCertificatesCommandStoreLocationDescription", AltName = nameof(IClientCertArgsWithStoreData.StoreLocation))]
#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member
public string StoreLocation { get; set; }
#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member

[Option(typeof(NuGetCommand), "ClientCertificatesCommandStoreNameDescription", AltName = nameof(IClientCertArgsWithStoreData.StoreName))]
#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member
public string StoreName { get; set; }
#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member

[Option(typeof(NuGetCommand), "ClientCertificatesCommandStorePasswordInClearTextDescription", AltName = nameof(IClientCertArgsWithFileData.StorePasswordInClearText))]
#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member
public bool StorePasswordInClearText { get; set; }
#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member

#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member
public override void ExecuteCommand()
#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member
{
var actionString = Arguments.FirstOrDefault() ?? "list";
switch (actionString.ToUpperInvariant())
Expand Down
Loading