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

Dotnet 5 support #163

Closed
rouke-broersma opened this issue Jan 11, 2021 · 2 comments · Fixed by #164
Closed

Dotnet 5 support #163

rouke-broersma opened this issue Jan 11, 2021 · 2 comments · Fixed by #164

Comments

@rouke-broersma
Copy link

rouke-broersma commented Jan 11, 2021

It seems that due to dotnet 5 no longer being called dotnet core the target framework parsing is broken.

See stryker-mutator/stryker-net#1369

System.IndexOutOfRangeException: Index was outside the bounds of the array.
at System.String.get_Chars(Int32 index)
at Buildalyzer.Environment.EnvironmentFactory.IsFrameworkTargetFramework(String targetFramework)
at Buildalyzer.Environment.EnvironmentFactory.b__11_0(String x)
at System.Linq.Enumerable.All[TSource](IEnumerable1 source, Func2 predicate)
at Buildalyzer.Environment.EnvironmentFactory.OnlyTargetsFramework(String targetFramework)
at Buildalyzer.Environment.EnvironmentFactory.GetBuildEnvironment(String targetFramework, EnvironmentOptions options)
at Buildalyzer.Environment.EnvironmentFactory.GetBuildEnvironment(String targetFramework)
at Buildalyzer.ProjectAnalyzer.Build(String targetFramework)
at Buildalyzer.ProjectAnalyzer.Build() 
@daveaglick
Copy link
Collaborator

Looks like there's actually a couple things going on here. Hopefully #164 resolves issues with identifying .NET 5, but the stack trace also suggests a bug in the code. Here's the code for IsFrameworkTargetFramework(string):

private bool IsFrameworkTargetFramework(string targetFramework) =>
    targetFramework.StartsWith("net", StringComparison.OrdinalIgnoreCase)
        && targetFramework.Length > 3
        && char.IsDigit(targetFramework[4]);

The stack trace suggests it's breaking on targetFramework[4] since that's the only array access. The bug is that we check the array length prior to the access, but I only check if the length is > 3 which would be a maximum index of 3 not 4. So in the case where the length is exactly 4 the maximum index is 3 and this code crashes.

When brings up another question for me - what was the value we were getting here for the target framework that was exactly 4 characters long. I suspect it was "net5" which is interesting in and of itself because the technically correct TFM for .NET 5 is "net5.0". If it had been "net5.0" this code actually would have worked, though for the wrong reasons! It would have checked if the 4th character, a ".", was a digit and failed - and that only would have worked because the other .NET Framework TFMs all have the versions without a ".".

tl;dr: This was a really interesting bug with several things I need to clean up here.

@slang25
Copy link
Contributor

slang25 commented Jan 12, 2021

Nice find, I didn't spot the off-by-one bug 🤦

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants