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

enable nullable for RuntimeFramework #4178

Merged
merged 4 commits into from Sep 21, 2022
Merged

enable nullable for RuntimeFramework #4178

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8379bea
enable nullable for RuntimeFramework
SimonCropp Sep 13, 2022
57109d9
.
SimonCropp Sep 14, 2022
c504c53
Merge remote-tracking branch 'upstream/master' into enable-nullable-f…
SimonCropp Sep 18, 2022
97fe0b3
Update Directory.Build.props
SimonCropp Sep 18, 2022
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
2 changes: 1 addition & 1 deletion src/NUnitFramework/Directory.Build.props
View file
Open in desktop
@@ -1,7 +1,7 @@
<Project>

<PropertyGroup>
<LangVersion Condition="'$(MSBuildProjectExtension)' == '.csproj'">8</LangVersion>
<LangVersion Condition="'$(MSBuildProjectExtension)' == '.csproj'">9</LangVersion>
<Features>strict</Features>
<SignAssembly>true</SignAssembly>
<AssemblyOriginatorKeyFile>..\..\nunit.snk</AssemblyOriginatorKeyFile>
Expand Down
110 changes: 0 additions & 110 deletions ...itFramework/framework/Compatibility/System.Diagnostics.CodeAnalysis/NullableAttributes.cs
View file
Open in desktop

This file was deleted.

33 changes: 14 additions & 19 deletions src/NUnitFramework/framework/Internal/RuntimeFramework.cs
View file
Open in desktop
@@ -1,13 +1,13 @@
// Copyright (c) Charlie Poole, Rob Prouse and Contributors. MIT License - see LICENSE.txt

#nullable enable

using System;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Reflection;
using System.Runtime.CompilerServices;
#if NETSTANDARD2_0
using System.Runtime.Versioning;
#else
using Microsoft.Win32;
using System.Runtime.CompilerServices;
#endif

namespace NUnit.Framework.Internal
Expand All @@ -34,8 +34,8 @@ public sealed class RuntimeFramework

private static readonly Lazy<RuntimeFramework> currentFramework = new Lazy<RuntimeFramework>(() =>
{
Type monoRuntimeType = null;
Type monoTouchType = null;
Type? monoRuntimeType = null;
Type? monoTouchType = null;

try
{
Expand Down Expand Up @@ -105,7 +105,7 @@ public sealed class RuntimeFramework

if (isMono)
{
MethodInfo getDisplayNameMethod = monoRuntimeType.GetMethod(
MethodInfo? getDisplayNameMethod = monoRuntimeType!.GetMethod(
"GetDisplayName", BindingFlags.Static | BindingFlags.NonPublic | BindingFlags.DeclaredOnly | BindingFlags.ExactBinding);
if (getDisplayNameMethod != null)
currentFramework.DisplayName = (string)getDisplayNameMethod.Invoke(null, Array.Empty<object>());
Expand Down Expand Up @@ -137,6 +137,8 @@ public RuntimeFramework( RuntimeType runtime, Version version)
DisplayName = GetDefaultDisplayName(runtime, version);
}

[MemberNotNull(nameof(FrameworkVersion))]
[MemberNotNull(nameof(ClrVersion))]
private void InitFromFrameworkVersion(Version version)
{
FrameworkVersion = ClrVersion = version;
Expand Down Expand Up @@ -191,6 +193,8 @@ private static void ThrowInvalidFrameworkVersion(Version version)
throw new ArgumentException("Unknown framework version " + version, nameof(version));
}

[MemberNotNull(nameof(FrameworkVersion))]
[MemberNotNull(nameof(ClrVersion))]
private void InitFromClrVersion(Version version)
{
FrameworkVersion = new Version(version.Major, version.Minor);
Expand All @@ -210,13 +214,7 @@ private void InitFromClrVersion(Version version)
/// Static method to return a RuntimeFramework object
/// for the framework that is currently in use.
/// </summary>
public static RuntimeFramework CurrentFramework
{
get
{
return currentFramework.Value;
}
}
public static RuntimeFramework CurrentFramework => currentFramework.Value;
SimonCropp marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// The type of this runtime framework
Expand All @@ -237,10 +235,7 @@ public static RuntimeFramework CurrentFramework
/// Return true if any CLR version may be used in
/// matching this RuntimeFramework object.
/// </summary>
public bool AllowAnyVersion
{
get { return ClrVersion == DefaultVersion; }
}
public bool AllowAnyVersion => ClrVersion == DefaultVersion;

/// <summary>
/// Returns the Display name for this framework
Expand Down Expand Up @@ -352,7 +347,7 @@ private static bool IsNetCore()
#if NETSTANDARD2_0
// Mono versions will throw a TypeLoadException when attempting to run the internal method, so we wrap it in a try/catch
// block to stop any inlining in release builds and check whether the type exists
Type runtimeInfoType = Type.GetType("System.Runtime.InteropServices.RuntimeInformation,System.Runtime.InteropServices.RuntimeInformation", false);
Type? runtimeInfoType = Type.GetType("System.Runtime.InteropServices.RuntimeInformation,System.Runtime.InteropServices.RuntimeInformation", false);
if (runtimeInfoType != null)
{
try
Expand Down
4 changes: 1 addition & 3 deletions src/NUnitFramework/framework/nunit.framework.csproj
View file
Open in desktop
Expand Up @@ -8,9 +8,7 @@

<ItemGroup>
<PackageReference Include="Microsoft.NETFramework.ReferenceAssemblies" Version="1.0.0" />
</ItemGroup>

<ItemGroup>
<PackageReference Include="Nullable" Version="1.3.1" PrivateAssets="all" />
Copy link
Member

Choose a reason for hiding this comment

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

Oh, now I understand. We've swapped out our own "shape" and copy for this. I'm unsure what the decision process is for taking on new dependencies, but I do like your inclusion of PrivateAssets here.
@jnm2 I think you did the initial pass in making some things nullable. Can I ask you your thoughts on this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

note that on "taking on new dependencies", Nullable is a source only package. So it has no effect on downstreams

Copy link
Member

Choose a reason for hiding this comment

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

Ah, this one confused me. I had to go out to the NuGet package and read the description to understand, "This package does not contain any compiled binaries, but instead adds the attribute classes as C# source code to your project." A note as such in the PR would have been helpful.

<PackageReference Include="System.Runtime" version="4.3.1" />
</ItemGroup>

Expand Down