managed-commons / SvgNet

Fork of the SVG library for .NET that makes a GdiGraphics that "draws" on a SVG model
BSD 3-Clause "New" or "Revised" License
84 stars 37 forks source link

Avoid declaring extension methods in the System namespace #72

Closed glopesdev closed 1 year ago

glopesdev commented 1 year ago

We're having ambiguous method overload resolution errors at compile-time in Unity scripts and elsewhere that we traced down to the Contains extension method which is defined below under the System namespace:

https://github.com/managed-commons/SvgNet/blob/5d69f7df5b7a0aa63503eafd1d17f51f5ae53903/SvgNet/Extensions/StringExtensions.cs#L9

It seems like this approach is shared by all extension methods in SvgNet. This seems like a problematic approach for extension methods, since System is the main top-level namespace used by almost all C# code. Having extension methods declared there can cause very hard to resolve name-clashes as soon as any third party code redeclares the same extension method in any other namespace.

Ideally these would be moved to the SvgNet namespace where they can be imported as needed. The latest versions of Visual Studio are quite good at picking up extension methods in all referenced assemblies, regardless of whether the namespace has been imported. Assuming this approach was adopted to make the extension methods easier to find, this wouldn't be a problem anymore.

monoman commented 1 year ago

Moved just the offending method (needed only in older versions of .NET and .NET Framework) to another extension class in the SvgNet namespace (version v3.3.4), this will cause the least ABI breakage but hopefully still avoiding the collision on Unity.