From 148ee1b4f7f064f017b2c7b7333e0c5168bb5509 Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Fri, 30 Dec 2022 15:25:05 -0800 Subject: [PATCH] Convert OpenXmlAttribute to readonly (#1282) --- CHANGELOG.md | 1 + .../OpenXmlAttribute.cs | 61 ++++++------------- .../OpenXmlCompositeElementTestClass.cs | 12 +--- .../ofapiTest/OpenXmlElementTest.cs | 26 ++++---- 4 files changed, 34 insertions(+), 66 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f0e76c77..feac46b76 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Breaking change - Removed `OpenXmlSimpleType.TextValue`. This property was never meant to be used externally - Core infrastructure is now contained in a new package DocumentFormat.OpenXml.Framework. Typed classes are still in DocumentFormat.OpenXml. This means that you may reference DocumentFormat.OpenXml and still compile the same types, but if you want a smaller package, you may rely on just the framework package. +- Removed mutable properties on OpenXmlAttribute and marked as `readonly` ## [2.20.0] diff --git a/src/DocumentFormat.OpenXml.Framework/OpenXmlAttribute.cs b/src/DocumentFormat.OpenXml.Framework/OpenXmlAttribute.cs index 9eb70f9f5..5d921495f 100644 --- a/src/DocumentFormat.OpenXml.Framework/OpenXmlAttribute.cs +++ b/src/DocumentFormat.OpenXml.Framework/OpenXmlAttribute.cs @@ -11,18 +11,13 @@ namespace DocumentFormat.OpenXml /// /// Represents an Open XML attribute. /// - public struct OpenXmlAttribute : IEquatable + public readonly struct OpenXmlAttribute : IEquatable { - private const string ObsoleteMessage = "OpenXmlAttribute is a struct so setters may not behave as you might expect. Mutable setters will be removed in a future version."; - - private string? _value; - private string _prefix; - internal OpenXmlAttribute(in OpenXmlQualifiedName qname, string prefix, string? value) { QName = qname; - _value = value; - _prefix = prefix; + Value = value; + Prefix = prefix; } /// @@ -41,8 +36,8 @@ public OpenXmlAttribute(string qualifiedName, string namespaceUri, string? value var parsed = PrefixName.Parse(qualifiedName); QName = new(namespaceUri, parsed.Name); - _prefix = parsed.Prefix; - _value = value; + Prefix = parsed.Prefix; + Value = value; } /// @@ -60,49 +55,29 @@ public OpenXmlAttribute(string prefix, string localName, string namespaceUri, st } QName = new(namespaceUri, localName); - _prefix = prefix; - _value = value; + Prefix = prefix; + Value = value; } /// - /// Gets or sets the namespace URI of the current attribute. + /// Gets the namespace URI of the current attribute. /// - public string NamespaceUri - { - get => QName.Namespace.Uri; - [Obsolete(ObsoleteMessage)] - set => QName = new(value, LocalName); - } + public string NamespaceUri => QName.Namespace.Uri; /// - /// Gets or sets the local name of the attribute. + /// Gets the local name of the attribute. /// - public string LocalName - { - get => QName.Name; - [Obsolete(ObsoleteMessage)] - set => QName = new(QName.Namespace, value); - } + public string LocalName => QName.Name; /// - /// Gets or sets the namespace prefix of the current attribute. + /// Gets the namespace prefix of the current attribute. /// - public string Prefix - { - get => _prefix; - [Obsolete(ObsoleteMessage)] - set => _prefix = value; - } + public string Prefix { get; } /// - /// Gets or sets the text value of the attribute. + /// Gets the text value of the attribute. /// - public string? Value - { - get => _value; - [Obsolete(ObsoleteMessage)] - set => _value = value; - } + public string? Value { get; } /// /// Gets the qualified name of the attribute. @@ -114,7 +89,7 @@ public string? Value /// public XName XName => XName.Get(LocalName, QName.Namespace.Uri); - internal OpenXmlQualifiedName QName { get; private set; } + internal OpenXmlQualifiedName QName { get; } /// /// Determines if this instance of an OpenXmlAttribute structure is equal to the specified instance of an OpenXmlAttribute structure. @@ -124,7 +99,7 @@ public string? Value public bool Equals(OpenXmlAttribute other) => string.Equals(Value, other.Value, StringComparison.Ordinal) && QName.Equals(other.QName) - && string.Equals(_prefix, other._prefix, StringComparison.Ordinal); + && string.Equals(Prefix, other.Prefix, StringComparison.Ordinal); /// /// Determines if two instances of OpenXmlAttribute structures are equal. @@ -162,7 +137,7 @@ public override int GetHashCode() hashcode.Add(Value, StringComparer.Ordinal); hashcode.Add(QName); - hashcode.Add(_prefix, StringComparer.Ordinal); + hashcode.Add(Prefix, StringComparer.Ordinal); return hashcode.ToHashCode(); } diff --git a/test/DocumentFormat.OpenXml.Tests/OpenXmlDomTest/OpenXmlCompositeElementTestClass.cs b/test/DocumentFormat.OpenXml.Tests/OpenXmlDomTest/OpenXmlCompositeElementTestClass.cs index b538ae9ab..b33bb2ede 100644 --- a/test/DocumentFormat.OpenXml.Tests/OpenXmlDomTest/OpenXmlCompositeElementTestClass.cs +++ b/test/DocumentFormat.OpenXml.Tests/OpenXmlDomTest/OpenXmlCompositeElementTestClass.cs @@ -587,15 +587,9 @@ public void OpenXmlAttributeValueTypeTest() Log.Comment("Constructing OpenXmlAttribute w:rsidR and assigning it to another variable..."); var wrsidR = new OpenXmlAttribute("w", "rsidR", "http://schemas.openxmlformats.org/wordprocessingml/2006/main", "00B327F7"); - var wrsidP = wrsidR; - Log.VerifyTrue(wrsidP == wrsidR, "The assigned OpenXmlAttribute variable is NOT equal to original one."); - -#pragma warning disable CS0618 // Type or member is obsolete - wrsidP.LocalName = "rsidP"; - wrsidP.Value = "00EC35BB"; -#pragma warning restore CS0618 // Type or member is obsolete - Log.Comment("Assigned new LocalName: {0} with ByValue: {1} and comparing it with original w:rsidR", wrsidP.LocalName, wrsidP.Value); - Log.VerifyFalse(wrsidP == wrsidR, "The assigned OpenXmlAttribute variable IS equal to original one."); + var wrsidP = new OpenXmlAttribute("w", "rsidP", "http://schemas.openxmlformats.org/wordprocessingml/2006/main", "00EC35BB"); + + Assert.NotEqual(wrsidP, wrsidR); } [Fact] diff --git a/test/DocumentFormat.OpenXml.Tests/ofapiTest/OpenXmlElementTest.cs b/test/DocumentFormat.OpenXml.Tests/ofapiTest/OpenXmlElementTest.cs index c6867aec1..5bb6c1de3 100644 --- a/test/DocumentFormat.OpenXml.Tests/ofapiTest/OpenXmlElementTest.cs +++ b/test/DocumentFormat.OpenXml.Tests/ofapiTest/OpenXmlElementTest.cs @@ -433,7 +433,6 @@ public void DefaultOpenXmlAttributeTest() [Fact] public void OpenXmlAttributeTest() { -#pragma warning disable CS0618 // Type or member is obsolete var target = new OpenXmlAttribute("test", "http://test", "test", "value"); var other = new OpenXmlAttribute("test", "http://test", "test", "value"); @@ -444,8 +443,13 @@ public void OpenXmlAttributeTest() Assert.True(target.Equals((object)other)); Assert.True(Equals(target, other)); Assert.Equal(target.GetHashCode(), other.GetHashCode()); + } - other.Value = "other"; + [Fact] + public void OpenXmlAttributeTestDifferentValues() + { + var target = new OpenXmlAttribute("test", "http://test", "test", "value"); + var other = new OpenXmlAttribute("test", "http://test", "test", "other"); Assert.NotEqual(target, other); Assert.False(target == other); @@ -454,18 +458,13 @@ public void OpenXmlAttributeTest() Assert.False(target.Equals((object)other)); Assert.False(Equals(target, other)); Assert.NotEqual(target.GetHashCode(), other.GetHashCode()); + } - other.Value = "value"; - - Assert.Equal(target, other); - Assert.True(target == other); - Assert.False(target != other); - Assert.True(target.Equals(other)); - Assert.True(target.Equals((object)other)); - Assert.True(Equals(target, other)); - Assert.Equal(target.GetHashCode(), other.GetHashCode()); - - other.Prefix = "t"; + [Fact] + public void OpenXmlAttributeTestDifferentPrefix() + { + var target = new OpenXmlAttribute("test", "http://test", "test", "value"); + var other = new OpenXmlAttribute("t", "http://test", "test", "value"); Assert.NotEqual(target, other); Assert.False(target == other); @@ -474,7 +473,6 @@ public void OpenXmlAttributeTest() Assert.False(target.Equals((object)other)); Assert.False(Equals(target, other)); Assert.NotEqual(target.GetHashCode(), other.GetHashCode()); -#pragma warning restore CS0618 // Type or member is obsolete } ///