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

Enable Code Quality rule CA1851 #20996

Merged
merged 2 commits into from
Aug 20, 2023
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
6 changes: 6 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -834,6 +834,12 @@ dotnet_diagnostic.CA1849.severity = warning
# Prefer static HashData method over ComputeHash. (Not available on mono)
dotnet_diagnostic.CA1850.severity = none

# Possible multiple enumerations of IEnumerable collection.
#dotnet_code_quality.CA1851.enumeration_methods =
dotnet_code_quality.CA1851.linq_chain_methods = M:OpenRA.Traits.IRenderModifier.Modify*
dotnet_code_quality.CA1851.assume_method_enumerates_parameters = true
dotnet_diagnostic.CA1851.severity = warning

# Seal internal types.
dotnet_diagnostic.CA1852.severity = warning

Expand Down
2 changes: 1 addition & 1 deletion OpenRA.Game/Exts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ public static T RandomOrDefault<T>(this IEnumerable<T> ts, MersenneTwister r)

static T Random<T>(IEnumerable<T> ts, MersenneTwister r, bool throws)
{
var xs = ts as ICollection<T>;
var xs = ts as IReadOnlyCollection<T>;
xs ??= ts.ToList();
if (xs.Count == 0)
{
Expand Down
5 changes: 3 additions & 2 deletions OpenRA.Game/Game.cs
Original file line number Diff line number Diff line change
Expand Up @@ -523,10 +523,11 @@ static string ChooseShellmap()
.Where(m => m.Status == MapStatus.Available && m.Visibility.HasFlag(MapVisibility.Shellmap))
.Select(m => m.Uid);

if (!shellmaps.Any())
var shellmap = shellmaps.RandomOrDefault(CosmeticRandom);
if (shellmap == null)
throw new InvalidDataException("No valid shellmaps available");

return shellmaps.Random(CosmeticRandom);
return shellmap;
}

public static void SwitchToExternalMod(ExternalMod mod, string[] launchArguments = null, Action onFailed = null)
Expand Down
4 changes: 3 additions & 1 deletion OpenRA.Game/GameRules/ActorInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ public IEnumerable<TraitInfo> TraitsInConstructOrder()

// Continue resolving traits as long as possible.
// Each time we resolve some traits, this means dependencies for other traits may then be possible to satisfy in the next pass.
#pragma warning disable CA1851 // Possible multiple enumerations of 'IEnumerable' collection
var readyToResolve = more.ToList();
while (readyToResolve.Count != 0)
{
Expand All @@ -138,6 +139,7 @@ public IEnumerable<TraitInfo> TraitsInConstructOrder()
readyToResolve.Clear();
readyToResolve.AddRange(more);
}
#pragma warning restore CA1851

if (unresolved.Count != 0)
{
Expand Down Expand Up @@ -185,7 +187,7 @@ public static IEnumerable<Type> OptionalPrerequisitesOf(TraitInfo info)
public bool HasTraitInfo<T>() where T : ITraitInfoInterface { return traits.Contains<T>(); }
public T TraitInfo<T>() where T : ITraitInfoInterface { return traits.Get<T>(); }
public T TraitInfoOrDefault<T>() where T : ITraitInfoInterface { return traits.GetOrDefault<T>(); }
public IEnumerable<T> TraitInfos<T>() where T : ITraitInfoInterface { return traits.WithInterface<T>(); }
public IReadOnlyCollection<T> TraitInfos<T>() where T : ITraitInfoInterface { return traits.WithInterface<T>(); }

public BitSet<TargetableType> GetAllTargetTypes()
{
Expand Down
2 changes: 1 addition & 1 deletion OpenRA.Game/Graphics/ChromeProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ public static Sprite[] TryGetPanelImages(string collectionName)
{
// PERF: We don't need to search for images if there are no definitions.
// PERF: It's more efficient to send an empty array rather than an array of 9 nulls.
if (!collection.Regions.Any())
if (collection.Regions.Count == 0)
return Array.Empty<Sprite>();

// Support manual definitions for unusual dialog layouts
Expand Down
7 changes: 5 additions & 2 deletions OpenRA.Game/Graphics/Viewport.cs
Original file line number Diff line number Diff line change
Expand Up @@ -299,10 +299,13 @@ IEnumerable<MPos> CandidateMouseoverCells(int2 world)

public void Center(IEnumerable<Actor> actors)
{
if (!actors.Any())
var actorsCollection = actors as IReadOnlyCollection<Actor>;
actorsCollection ??= actors.ToList();

if (actorsCollection.Count == 0)
return;

Center(actors.Select(a => a.CenterPosition).Average());
Center(actorsCollection.Select(a => a.CenterPosition).Average());
}

public void Center(WPos pos)
Expand Down
2 changes: 1 addition & 1 deletion OpenRA.Game/Map/ActorReference.cs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ public void Add(ActorInit init)
return removed;
}

public IEnumerable<T> GetAll<T>() where T : ActorInit
public IReadOnlyCollection<T> GetAll<T>() where T : ActorInit
{
return initDict.Value.WithInterface<T>();
}
Expand Down
5 changes: 2 additions & 3 deletions OpenRA.Game/Map/CellRegion.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;

namespace OpenRA
{
Expand Down Expand Up @@ -64,9 +63,9 @@ public static CellRegion Expand(CellRegion region, int cordon)
}

/// <summary>Returns the minimal region that covers at least the specified cells.</summary>
public static CellRegion BoundingRegion(MapGridType shape, IEnumerable<CPos> cells)
public static CellRegion BoundingRegion(MapGridType shape, IReadOnlyCollection<CPos> cells)
{
if (cells == null || !cells.Any())
if (cells == null || cells.Count == 0)
throw new ArgumentException("cells must not be null or empty.", nameof(cells));

var minU = int.MaxValue;
Expand Down
8 changes: 6 additions & 2 deletions OpenRA.Game/Network/SyncReport.cs
Original file line number Diff line number Diff line change
Expand Up @@ -201,8 +201,12 @@ struct EffectReport
public TypeInfo(Type type)
{
const BindingFlags Flags = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance;
var fields = type.GetFields(Flags).Where(fi => !fi.IsLiteral && !fi.IsStatic && fi.HasAttribute<SyncAttribute>());
var properties = type.GetProperties(Flags).Where(pi => pi.HasAttribute<SyncAttribute>());
var fields = type.GetFields(Flags)
.Where(fi => !fi.IsLiteral && !fi.IsStatic && fi.HasAttribute<SyncAttribute>())
.ToList();
var properties = type.GetProperties(Flags)
.Where(pi => pi.HasAttribute<SyncAttribute>())
.ToList();

foreach (var prop in properties)
if (!prop.CanRead || prop.GetIndexParameters().Length > 0)
Expand Down
4 changes: 2 additions & 2 deletions OpenRA.Game/ObjectCreator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,8 @@ public Type FindType(string className)
public ConstructorInfo GetCtor(Type type)
{
var flags = BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.Instance;
var ctors = type.GetConstructors(flags).Where(x => x.HasAttribute<UseCtorAttribute>());
if (ctors.Count() > 1)
var ctors = type.GetConstructors(flags).Where(x => x.HasAttribute<UseCtorAttribute>()).ToList();
RoosterDragon marked this conversation as resolved.
Show resolved Hide resolved
if (ctors.Count > 1)
throw new InvalidOperationException("ObjectCreator: UseCtor on multiple constructors; invalid.");
return ctors.FirstOrDefault();
}
Expand Down
2 changes: 1 addition & 1 deletion OpenRA.Game/Player.cs
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ static FactionInfo ResolveFaction(World world, string factionName, MersenneTwist

static FactionInfo ResolveDisplayFaction(World world, string factionName)
{
var factions = world.Map.Rules.Actors[SystemActors.World].TraitInfos<FactionInfo>().ToArray();
var factions = world.Map.Rules.Actors[SystemActors.World].TraitInfos<FactionInfo>();

return factions.FirstOrDefault(f => f.InternalName == factionName) ?? factions.First();
}
Expand Down
83 changes: 59 additions & 24 deletions OpenRA.Game/Primitives/TypeDictionary.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,20 @@
using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;

namespace OpenRA.Primitives
{
public class TypeDictionary : IEnumerable<object>
{
static readonly Func<Type, List<object>> CreateList = type => new List<object>();
readonly Dictionary<Type, List<object>> data = new();
static readonly Func<Type, ITypeContainer> CreateTypeContainer = t =>
(ITypeContainer)typeof(TypeContainer<>).MakeGenericType(t).GetConstructor(Type.EmptyTypes).Invoke(null);

readonly Dictionary<Type, ITypeContainer> data = new();

ITypeContainer InnerGet(Type t)
{
return data.GetOrAdd(t, CreateTypeContainer);
}

public void Add(object val)
{
Expand All @@ -33,7 +39,7 @@ public void Add(object val)

void InnerAdd(Type t, object val)
{
data.GetOrAdd(t, CreateList).Add(val);
InnerGet(t).Add(val);
}

public bool Contains<T>()
Expand All @@ -48,35 +54,33 @@ public bool Contains(Type t)

public T Get<T>()
{
return (T)Get(typeof(T), true);
return Get<T>(true);
}

public T GetOrDefault<T>()
{
var result = Get(typeof(T), false);
if (result == null)
return default;
return (T)result;
return Get<T>(false);
}

object Get(Type t, bool throwsIfMissing)
T Get<T>(bool throwsIfMissing)
{
if (!data.TryGetValue(t, out var ret))
if (!data.TryGetValue(typeof(T), out var container))
RoosterDragon marked this conversation as resolved.
Show resolved Hide resolved
{
if (throwsIfMissing)
throw new InvalidOperationException($"TypeDictionary does not contain instance of type `{t}`");
return null;
throw new InvalidOperationException($"TypeDictionary does not contain instance of type `{typeof(T)}`");
return default;
}

if (ret.Count > 1)
throw new InvalidOperationException($"TypeDictionary contains multiple instances of type `{t}`");
return ret[0];
var list = ((TypeContainer<T>)container).Objects;
if (list.Count > 1)
throw new InvalidOperationException($"TypeDictionary contains multiple instances of type `{typeof(T)}`");
return list[0];
}

public IEnumerable<T> WithInterface<T>()
public IReadOnlyCollection<T> WithInterface<T>()
{
if (data.TryGetValue(typeof(T), out var objs))
return objs.Cast<T>();
if (data.TryGetValue(typeof(T), out var container))
return ((TypeContainer<T>)container).Objects;
return Array.Empty<T>();
}

Expand All @@ -92,18 +96,19 @@ public void Remove<T>(T val)

void InnerRemove(Type t, object val)
{
if (!data.TryGetValue(t, out var objs))
if (!data.TryGetValue(t, out var container))
return;
objs.Remove(val);
if (objs.Count == 0)

container.Remove(val);
if (container.Count == 0)
data.Remove(t);
}

public void TrimExcess()
{
data.TrimExcess();
foreach (var objs in data.Values)
objs.TrimExcess();
foreach (var t in data.Keys)
InnerGet(t).TrimExcess();
}

public IEnumerator<object> GetEnumerator()
Expand All @@ -115,6 +120,36 @@ IEnumerator IEnumerable.GetEnumerator()
{
return GetEnumerator();
}

interface ITypeContainer
{
int Count { get; }
void Add(object value);
void Remove(object value);
void TrimExcess();
}

sealed class TypeContainer<T> : ITypeContainer
{
public List<T> Objects { get; } = new List<T>();

public int Count => Objects.Count;

public void Add(object value)
{
Objects.Add((T)value);
}

public void Remove(object value)
{
Objects.Remove((T)value);
}

public void TrimExcess()
{
Objects.TrimExcess();
}
}
}

public static class TypeExts
Expand Down
5 changes: 1 addition & 4 deletions OpenRA.Game/SelectableExts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,7 @@ static int BaseSelectionPriority(ISelectableInfo info, Modifiers modifiers)

public static Actor WithHighestSelectionPriority(this IEnumerable<ActorBoundsPair> actors, int2 selectionPixel, Modifiers modifiers)
{
if (!actors.Any())
return null;

return actors.MaxBy(a => CalculateActorSelectionPriority(a.Actor.Info, a.Bounds, selectionPixel, modifiers)).Actor;
return actors.MaxByOrDefault(a => CalculateActorSelectionPriority(a.Actor.Info, a.Bounds, selectionPixel, modifiers)).Actor;
RoosterDragon marked this conversation as resolved.
Show resolved Hide resolved
}

public static FrozenActor WithHighestSelectionPriority(this IEnumerable<FrozenActor> actors, int2 selectionPixel, Modifiers modifiers)
Expand Down
2 changes: 1 addition & 1 deletion OpenRA.Game/Support/AssemblyLoader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ static IEnumerable<string> GetRids(RuntimeFallbacks runtimeGraph)
return Enumerable.Concat(new[] { runtimeGraph.Runtime }, runtimeGraph?.Fallbacks ?? Enumerable.Empty<string>());
}

static IEnumerable<string> SelectAssets(IEnumerable<string> rids, IEnumerable<RuntimeAssetGroup> groups)
static IEnumerable<string> SelectAssets(IEnumerable<string> rids, IReadOnlyCollection<RuntimeAssetGroup> groups)
{
foreach (var rid in rids)
{
Expand Down
2 changes: 1 addition & 1 deletion OpenRA.Game/Traits/TraitsInterfaces.cs
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ public interface ISelectableInfo : ITraitInfoInterface
public interface ISelection
{
int Hash { get; }
IEnumerable<Actor> Actors { get; }
IReadOnlyCollection<Actor> Actors { get; }

void Add(Actor a);
void Remove(Actor a);
Expand Down
10 changes: 5 additions & 5 deletions OpenRA.Game/WPos.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

using System;
using System.Collections.Generic;
using System.Linq;
using Eluant;
using Eluant.ObjectBinding;
using OpenRA.Scripting;
Expand Down Expand Up @@ -139,20 +138,21 @@ public static class IEnumerableExtensions
{
public static WPos Average(this IEnumerable<WPos> source)
{
var length = source.Count();
if (length == 0)
return WPos.Zero;

var length = 0;
var x = 0L;
var y = 0L;
var z = 0L;
foreach (var pos in source)
{
length++;
x += pos.X;
y += pos.Y;
z += pos.Z;
}

if (length == 0)
return WPos.Zero;

x /= length;
y /= length;
z /= length;
Expand Down
3 changes: 1 addition & 2 deletions OpenRA.Mods.Cnc/Activities/Teleport.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#endregion

using System;
using System.Linq;
using OpenRA.Activities;
using OpenRA.Mods.Cnc.Traits;
using OpenRA.Mods.Common.Traits;
Expand Down Expand Up @@ -120,7 +119,7 @@ public override bool Tick(Actor self)
if (teleporter == null)
return null;

var restrictTo = maximumDistance == null ? null : self.World.Map.FindTilesInCircle(self.Location, maximumDistance.Value);
var restrictTo = maximumDistance == null ? null : self.World.Map.FindTilesInCircle(self.Location, maximumDistance.Value).ToHashSet();

if (maximumDistance != null)
destination = restrictTo.MinBy(x => (x - destination).LengthSquared);
Expand Down
2 changes: 1 addition & 1 deletion OpenRA.Mods.Cnc/Traits/GpsWatcher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ void RefreshGranted()
{
var wasGranted = Granted;
var wasGrantedAllies = GrantedAllies;
var allyWatchers = owner.World.ActorsWithTrait<GpsWatcher>().Where(kv => kv.Actor.Owner.IsAlliedWith(owner));
var allyWatchers = owner.World.ActorsWithTrait<GpsWatcher>().Where(kv => kv.Actor.Owner.IsAlliedWith(owner)).ToList();

Granted = actors.Count > 0 && Launched;
GrantedAllies = allyWatchers.Any(w => w.Trait.Granted);
Expand Down