Skip to content
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions eng/WpfArcadeSdk/tools/CodeAnalysis.targets
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@
<ItemGroup Condition="'$(IsTestProject)'!='true' AND '$(EnableAnalyzers)'=='true'">
<!-- Managed Code Reference analyzers -->
<PackageReference Include="Microsoft.DotNet.CodeAnalysis" Version="$(MicrosoftDotNetCodeAnalysisPackageVersion)" />
<PackageReference Include="Microsoft.CodeAnalysis.Common" Version="2.9.0" />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This shouldn't be needed and it conflicts with other PackageReference when enabling analyzers in any other project than System.Xaml.

<PackageReference Include="Microsoft.CodeAnalysis.FxCopAnalyzers" Version="2.6.3" />
<PackageReference Include="Microsoft.CodeAnalysis.FxCopAnalyzers" Version="3.3.1" />
<PackageReference Include="System.Runtime.Analyzers" Version="1.1.0" />
<PackageReference Include="System.Runtime.InteropServices.Analyzers" Version="1.1.0" />
</ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ internal bool EndsEscapeSequence(char ch)

internal bool Match(char start, char end)
{
return _endChars.IndexOf(end.ToString()) == _startChars.IndexOf(start.ToString());
return _endChars.IndexOf(end.ToString(), StringComparison.Ordinal) == _startChars.IndexOf(start.ToString(), StringComparison.Ordinal);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

}

internal string StartBracketCharacters
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public string Name
}
}

private string _name = null;
private string _name;
}
#endif

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,6 @@ public string Name
}

// The name of the property that is designated to accept the x:Uid value
private string _name = null;
private string _name;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,9 @@ public static ValueSerializer GetSerializerFor(PropertyDescriptor descriptor, IV
/// </summary>
protected Exception GetConvertToException(object value, Type destinationType)
{
if (destinationType == null)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one would already throw a NullReferenceException destinationType.FullName. By adding an explicit throw here, we make sure to throw early, with the right stacktrace and the right parameter name.

This one is technically a breaking change, the type of exception returned is not the same as before. Let me know if you would prefer to revert this change.

Copy link
Contributor

Choose a reason for hiding this comment

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

Whilst this is a breaking change indeed, these types of changes have been generally considered acceptable across the .NET ecosystem.
https://github.com/dotnet/runtime/blob/main/docs/coding-guidelines/breaking-changes.md#bucket-2-reasonable-grey-area

throw new ArgumentNullException(nameof(destinationType));

string text;
if (value == null)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ internal class ObjectWriterContext : XamlContext
{
private XamlContextStack<ObjectWriterFrame> _stack;

private object _rootInstance = null;
private object _rootInstance;

ServiceProviderContext _serviceProviderContext;
XamlRuntime _runtime;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ internal IList<XmlNsDefinition> NsDefs
}

// Note this, is the only dictionary that we synchronize, because XamlSchemaContext adds to it
private ConcurrentDictionary<string, IList<string>> _clrToXmlNs = null;
private ConcurrentDictionary<string, IList<string>> _clrToXmlNs;
internal ConcurrentDictionary<string, IList<string>> ClrToXmlNs
{
get
Expand All @@ -64,7 +64,7 @@ internal ICollection<AssemblyName> InternalsVisibleTo
}
}

private Dictionary<string, string> _oldToNewNs = null;
private Dictionary<string, string> _oldToNewNs;
internal Dictionary<string, string> OldToNewNs
{
get
Expand All @@ -77,7 +77,7 @@ internal Dictionary<string, string> OldToNewNs
}
}

private Dictionary<string, string> _prefixes = null;
private Dictionary<string, string> _prefixes;
internal Dictionary<string, string> Prefixes
{
get
Expand All @@ -90,7 +90,7 @@ internal Dictionary<string, string> Prefixes
}
}

private string _rootNamespace = null;
private string _rootNamespace;
internal string RootNamespace
{
get
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public NamespacePrefixLookup(out IEnumerable<NamespaceDeclaration> newNamespaces

#region INamespacePrefixLookup Members

private int n = 0;
private int n;
public string LookupPrefix(string ns)
{
// we really shouldn't generate extraneous new namespaces
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class SeenCtorDirectiveFlags
}

List<SeenCtorDirectiveFlags> _seenStack = new List<SeenCtorDirectiveFlags>();
int _startObjectDepth = 0;
int _startObjectDepth;

List<int> _moveList;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public XamlName(string prefix, string name)
public abstract string ScopedName { get; }

protected string _prefix;
protected string _namespace = null;
protected string _namespace;

public string Prefix { get { return _prefix; } }
public string Namespace { get { return _namespace; } }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -931,7 +931,7 @@ private XamlNode Logic_StartItemsProperty(XamlType collectionType)

#region Optimizations
private readonly XamlTypeName arrayType = new XamlTypeName(@"http://schemas.microsoft.com/winfx/2006/xaml", "Array");
private XamlType _arrayExtensionType = null;
private XamlType _arrayExtensionType;
private XamlType ArrayExtensionType
{
get
Expand All @@ -944,7 +944,7 @@ private XamlType ArrayExtensionType
}
}

private XamlMember _arrayTypeMember = null;
private XamlMember _arrayTypeMember;
private XamlMember ArrayTypeMember
{
get
Expand All @@ -957,7 +957,7 @@ private XamlMember ArrayTypeMember
}
}

private XamlMember _itemsTypeMember = null;
private XamlMember _itemsTypeMember;
private XamlMember ItemsTypeMember
{
get
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,14 @@ class XamlScanner
XamlScannerStack _scannerStack;
XamlParserContext _parserContext;

XamlText _accumulatedText = null;
XamlText _accumulatedText;
List<XamlAttribute> _attributes;
int _nextAttribute;
XamlScannerNode _currentNode;
Queue<XamlScannerNode> _readNodesQueue;
XamlXmlReaderSettings _settings;
XamlAttribute _typeArgumentAttribute;
bool _hasKeyAttribute = false;
bool _hasKeyAttribute;

internal XamlScanner(XamlParserContext context, XmlReader xmlReader, XamlXmlReaderSettings settings)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ public class XamlBackgroundReader : XamlReader, IXamlLineInfo
XamlWriter _writer;

bool _wrappedReaderHasLineInfo;
int _lineNumber=0;
int _linePosition=0;
int _lineNumber;
int _linePosition;

Thread _thread;
Exception _caughtException;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ namespace System.Xaml
public class XamlNodeList
{
List<XamlNode> _nodeList;
bool _readMode = false;
bool _readMode;
XamlWriter _writer;
bool _hasLineInfo;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

namespace System.Xaml.Schema
{
[Diagnostics.CodeAnalysis.SuppressMessage("Reliability", "CA2002:Do not lock on objects with weak identity", Justification = "This type is internal.")]
class TypeReflector : Reflector
{
private const XamlCollectionKind XamlCollectionKindInvalid = (XamlCollectionKind)byte.MaxValue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -994,7 +994,7 @@ public virtual void EnsureNoDuplicateNames(Stack<HashSet<string>> namesInCurrent
class ObjectMarkupInfo : ObjectOrValueMarkupInfo
{
List<MarkupInfo> properties = new List<MarkupInfo>();
bool? isAttributableMarkupExtension = null;
bool? isAttributableMarkupExtension;

public List<MarkupInfo> Properties { get { return properties; } }
public string Name { get; set; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -542,6 +542,9 @@ private ConcurrentDictionary<ReferenceEqualityTuple<MemberInfo, MemberInfo>, Xam

public virtual XamlType GetXamlType(Type type)
{
if (type == null)
Copy link
Contributor Author

@ThomasGoulet73 ThomasGoulet73 Jul 29, 2021

Choose a reason for hiding this comment

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

This one would succeed in XamlLanguage.TypeAlias(type) but throw in GetXamlType. By adding an explicit throw here, we make sure to throw early, with the right stacktrace and the right parameter name.

throw new ArgumentNullException(nameof(type));

return GetXamlType(type, XamlLanguage.TypeAlias(type));
}

Expand Down Expand Up @@ -683,7 +686,7 @@ internal virtual XamlMember GetAttachableEvent(string name, MethodInfo adder)
#region Settings

// Unchanging, initialized in ctor
private readonly XamlSchemaContextSettings _settings = null;
private readonly XamlSchemaContextSettings _settings;

public bool SupportMarkupExtensionsWithDuplicateArity
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

namespace System.Xaml
{
[Diagnostics.CodeAnalysis.SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope", Justification = "The IDisposable types in this class don't require dispose.")]
public static class XamlServices
{
// The main function is Load(XamlReader)
Expand Down