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

Code fix to add unsafe option to C# project #25875

Merged
merged 59 commits into from
Jan 16, 2019
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
59 commits
Select commit Hold shift + click to select a range
8c35eaf
Allowing ChangeCompilationOptions in VisualStudioWorkspace
Neme12 Apr 1, 2018
5f8c43d
This_workspace_does_not_support_updating_Visual_Basic_compilation_opt…
Neme12 Apr 1, 2018
6db55aa
CanApplyCompilationOptionChange
Neme12 Apr 1, 2018
ad48331
Adding AllowUnsafe option to TestWorkspace
Neme12 Apr 1, 2018
8cdfeda
Simplifying CreateCompilationOptionsElement
Neme12 Apr 1, 2018
dbfbdbe
Adding tests + implementation
Neme12 Apr 1, 2018
61001fb
Renaming ParseOptionsChangeAction
Neme12 Apr 1, 2018
cbd1e91
Localization
Neme12 Apr 1, 2018
f162695
Adding comment for missing fix all
Neme12 Apr 1, 2018
7b511a8
Simplifying enumeration of ConfigurationManager
Neme12 Apr 2, 2018
a934f22
Renaming
Neme12 Apr 2, 2018
6fd37d0
Updating namespaces
Neme12 Apr 2, 2018
197caa4
Changing comment for missing fix all
Neme12 Apr 2, 2018
1d0e4ac
Adding integration tests
Neme12 Apr 3, 2018
cce1de4
Fixing conflict
Neme12 Apr 4, 2018
b2a6df7
Using WpfFact for integration tests
Neme12 Apr 4, 2018
51f8175
Merge remote-tracking branch 'upstream/master' into allowUnsafe
Neme12 Apr 7, 2018
7003d31
Using XML checks for expected project file in tests
Neme12 Apr 7, 2018
c5275a0
Using IVsBuildPropertyStorage for CPS projects
Neme12 Apr 8, 2018
4ba8b42
Adding tests for CPS projects
Neme12 Apr 8, 2018
b0d60cf
Helper test methods
Neme12 Apr 8, 2018
4f90119
Adding test with default template for legacy project
Neme12 Apr 8, 2018
b56a801
Test cleanup
Neme12 Apr 8, 2018
06c8c5e
More test cleanup
Neme12 Apr 8, 2018
82c4342
Fixing trait
Neme12 Apr 8, 2018
50c3160
Using deconstruction
Neme12 Apr 12, 2018
01220e7
Fixing resource conflict
Neme12 Apr 20, 2018
28f3ee0
Addressing feedback
Neme12 Apr 20, 2018
1482394
Trying to fix flaky tests
Neme12 Apr 20, 2018
46e9abd
Trying different fix
Neme12 Apr 20, 2018
cdb4dcf
Simplifying configuration property access
Neme12 Apr 20, 2018
8ddad99
Merge remote-tracking branch 'upstream/master' into allowUnsafe
Neme12 May 26, 2018
613e513
Fix after merge
Neme12 May 26, 2018
fe460b1
Merging & fixing resx conflicts, updating VisualStudioWorkspaceImpl, …
Neme12 Sep 24, 2018
99046c6
Fixing namespace for integration tests
Neme12 Sep 24, 2018
139944e
Removing WaitForApplicationIdle
Neme12 Sep 24, 2018
2b7dd40
Making upgrade project integration tests future-proof
Neme12 Sep 25, 2018
461e397
Adding an explanatory comment to ProjectPropertyStorage
Neme12 Sep 25, 2018
48bb6d8
Fixing name violation
Neme12 Sep 25, 2018
9413f0d
Merge remote-tracking branch 'upstream/master' into allowUnsafe
Neme12 Sep 25, 2018
a930d92
Running integration tests that use a project template in the existing…
Neme12 Sep 26, 2018
6f3e906
Merge remote-tracking branch 'upstream/master' into allowUnsafe
Neme12 Oct 12, 2018
3bd3b59
Merge remote-tracking branch 'upstream/master' into allowUnsafe
Neme12 Oct 14, 2018
e203e94
Merge remote-tracking branch 'upstream/master' into allowUnsafe
Neme12 Oct 15, 2018
8d70818
Fixing merge conflicts
Neme12 Oct 20, 2018
7e2746d
Simplifying after merge
Neme12 Oct 20, 2018
87f68b1
Merge remote-tracking branch 'upstream/master' into allowUnsafe
Neme12 Dec 6, 2018
ec87d24
Moving IParseOptionsService & ICompilationOptionsService to Microsoft…
Neme12 Dec 6, 2018
b304660
Adding IParseOptionsChangingService and ICompilationOptionsChangingSe…
Neme12 Dec 6, 2018
2289a5f
Treating VB equally
Neme12 Dec 8, 2018
6070ba3
Save solution after creating it as a workaround for CPS bug
Neme12 Dec 10, 2018
b57a581
Merge remote-tracking branch 'upstream/master' into allowUnsafe
Neme12 Dec 11, 2018
7a5ff09
Embed interop types
Neme12 Dec 11, 2018
3c98747
Add exclusion for VSLangProj80
Neme12 Dec 16, 2018
37e9562
Adding VSLangProj & VSLangProj2
Neme12 Dec 17, 2018
40244f5
Fixing merge conflicts
Neme12 Jan 4, 2019
4a73fa0
Removing duplicate code after merge
Neme12 Jan 4, 2019
9d42e1a
Fixing language change implementation - WithLanguageVersion maps to S…
Neme12 Jan 4, 2019
0edab52
Merge remote-tracking branch 'dotnet/dev16.1-preview1' into allowUnsafe
jasonmalinowski Jan 16, 2019
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.Linq;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.UnsafeProject;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Test.Utilities;
using Xunit;

namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.Diagnostics.UnsafeProject
{
[Trait(Traits.Feature, Traits.Features.CodeActionsUnsafeProject)]
public class UnsafeProjectTests : AbstractCSharpDiagnosticProviderBasedUserDiagnosticTest
{
internal override (DiagnosticAnalyzer, CodeFixProvider) CreateDiagnosticProviderAndFixer(Workspace workspace)
=> (null, new CSharpUnsafeProjectCodeFixProvider());

private async Task TestAllowUnsafeEnabledIfDisabledAsync(string initialMarkup)
{
var parameters = new TestParameters();
using (var workspace = CreateWorkspaceFromOptions(initialMarkup, parameters))
{
var actions = await GetCodeActionsAsync(workspace, parameters);
var operations = await VerifyInputsAndGetOperationsAsync(0, actions, priority: null);

var appliedChanges = ApplyOperationsAndGetSolution(workspace, operations);
var oldSolution = appliedChanges.Item1;
var newSolution = appliedChanges.Item2;
Assert.True(((CSharpCompilationOptions)newSolution.Projects.Single().CompilationOptions).AllowUnsafe);
}

// no action offered if unsafe was already enabled
await TestMissingAsync(initialMarkup, new TestParameters(compilationOptions:
new CSharpCompilationOptions(outputKind: default, allowUnsafe: true)));
}

[Fact]
public async Task OnUnsafeClass()
{
await TestAllowUnsafeEnabledIfDisabledAsync(
@"
unsafe class [|C|] // The compiler reports this on the name, not the 'unsafe' keyword.
{
}");
}

[Fact]
public async Task OnUnsafeMethod()
{
await TestAllowUnsafeEnabledIfDisabledAsync(
@"
class C
{
unsafe void [|M|]()
{
}
}");
}

[Fact]
public async Task OnUnsafeLocalFunction()
{
await TestAllowUnsafeEnabledIfDisabledAsync(
@"
class C
{
void M()
{
unsafe void [|Function|]()
{
}
}
}");
}

[Fact]
public async Task OnUnsafeBlock()
{
await TestAllowUnsafeEnabledIfDisabledAsync(
@"
class C
{
void M()
{
[|unsafe|]
{
}
}
}");
}

[Fact]
public async Task NotInsideUnsafeBlock()
{
await TestMissingAsync(
@"
class C
{
void M()
{
unsafe
{
[|int * p;|]
}
}
}");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ public partial class TestWorkspace
private const string AliasAttributeName = "Alias";
private const string ProjectNameAttribute = "Name";
private const string CheckOverflowAttributeName = "CheckOverflow";
private const string AllowUnsafeAttributeName = "AllowUnsafe";

/// <summary>
/// Creates a single buffer in a workspace.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,7 @@ private static CompilationOptions CreateCompilationOptions(TestWorkspace workspa
var strongNameProvider = default(StrongNameProvider);
var delaySign = default(bool?);
var checkOverflow = false;
bool allowUnsafe = false;
Neme12 marked this conversation as resolved.
Show resolved Hide resolved

if (compilationOptionsElement != null)
{
Expand All @@ -483,6 +484,12 @@ private static CompilationOptions CreateCompilationOptions(TestWorkspace workspa
checkOverflow = (bool)checkOverflowAttribute;
}

var allowUnsafeAttribute = compilationOptionsElement.Attribute(AllowUnsafeAttributeName);
if (allowUnsafeAttribute != null)
{
allowUnsafe = (bool)allowUnsafeAttribute;
}

var reportDiagnosticAttribute = compilationOptionsElement.Attribute(ReportDiagnosticAttributeName);
if (reportDiagnosticAttribute != null)
{
Expand Down Expand Up @@ -529,7 +536,7 @@ private static CompilationOptions CreateCompilationOptions(TestWorkspace workspa

// VB needs Compilation.ParseOptions set (we do the same at the VS layer)
return language == LanguageNames.CSharp
? (CompilationOptions)new CSharpCompilationOptions(OutputKind.WindowsRuntimeMetadata)
? (CompilationOptions)new CSharpCompilationOptions(OutputKind.WindowsRuntimeMetadata, allowUnsafe: allowUnsafe)
: new VisualBasicCompilationOptions(OutputKind.WindowsRuntimeMetadata).WithGlobalImports(globalImports).WithRootNamespace(rootNamespace)
.WithParseOptions((VisualBasicParseOptions)parseOptions ?? VisualBasicParseOptions.Default);
}
Expand Down Expand Up @@ -557,6 +564,11 @@ private static CompilationOptions CreateCompilationOptions(TestWorkspace workspa
.WithDelaySign(delaySign)
.WithOverflowChecks(checkOverflow);

if (language == LanguageNames.CSharp)
{
compilationOptions = ((CSharpCompilationOptions)compilationOptions).WithAllowUnsafe(allowUnsafe);
}

if (language == LanguageNames.VisualBasic)
{
// VB needs Compilation.ParseOptions set (we do the same at the VS layer)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,15 @@ private static XAttribute CreateDocumentationModeAttribute(ParseOptions parseOpt

private static XElement CreateCompilationOptionsElement(CompilationOptions options)
{
XElement element = null;
if (options is Microsoft.CodeAnalysis.VisualBasic.VisualBasicCompilationOptions vbOptions)
var element = new XElement(CompilationOptionsElementName);

if (options is CodeAnalysis.CSharp.CSharpCompilationOptions csOptions)
{
element.SetAttributeValue(AllowUnsafeAttributeName, csOptions.AllowUnsafe);
}
else if (options is CodeAnalysis.VisualBasic.VisualBasicCompilationOptions vbOptions)
{
element = new XElement(CompilationOptionsElementName,
vbOptions.GlobalImports.AsEnumerable().Select(i => new XElement(GlobalImportElementName, i.Name)));
element.Add(vbOptions.GlobalImports.AsEnumerable().Select(i => new XElement(GlobalImportElementName, i.Name)));

if (vbOptions.RootNamespace != null)
{
Expand All @@ -90,7 +94,6 @@ private static XElement CreateCompilationOptionsElement(CompilationOptions optio

if (options.CheckOverflow)
{
element = element ?? new XElement(CompilationOptionsElementName);
element.SetAttributeValue(CheckOverflowAttributeName, true);
}

Expand Down
20 changes: 19 additions & 1 deletion src/Features/CSharp/Portable/CSharpFeaturesResources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/Features/CSharp/Portable/CSharpFeaturesResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -524,4 +524,7 @@
<data name="Convert_for_to_foreach" xml:space="preserve">
<value>Convert 'for' to 'foreach'</value>
</data>
<data name="Allow_unsafe_code_in_this_project" xml:space="preserve">
<value>Allow unsafe code in this project</value>
Neme12 marked this conversation as resolved.
Show resolved Hide resolved
</data>
</root>
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.Collections.Immutable;
using System.Composition;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.UpgradeProject;

namespace Microsoft.CodeAnalysis.CSharp.UnsafeProject
{
[ExportCodeFixProvider(LanguageNames.CSharp), Shared]
internal class CSharpUnsafeProjectCodeFixProvider : CodeFixProvider
{
private const string CS0227 = nameof(CS0227); // error CS0227: Unsafe code may only appear if compiling with /unsafe

public override ImmutableArray<string> FixableDiagnosticIds { get; } =
ImmutableArray.Create(CS0227);

public override FixAllProvider GetFixAllProvider()
{
// Fix all could be implemented, but doesn't seem as important because for this to be useful,
Neme12 marked this conversation as resolved.
Show resolved Hide resolved
// the user would have to have erroneous unsafe blocks in several projects, which sounds really unlikely.
// And after all, unsafe code itself is and should be an edge case - do we want to make it easier
// to convert to it on a larger scale?
// If we do this, we should create a custom FixAllProvider that only supports FixAllScope.Solution
// since Document and Project don't really make sense - the action would always be the same as for fix one
// and having these extra options would only be confusing.
return null;
}

public override Task RegisterCodeFixesAsync(CodeFixContext context)
{
context.RegisterCodeFix(new ProjectOptionsChangeAction(CSharpFeaturesResources.Allow_unsafe_code_in_this_project,
_ => Task.FromResult(AllowUnsafeOnProject(context.Document.Project))), context.Diagnostics);
return Task.CompletedTask;
}

private static Solution AllowUnsafeOnProject(Project project)
{
var compilationOptions = (CSharpCompilationOptions)project.CompilationOptions;
return project.Solution.WithProjectCompilationOptions(project.Id, compilationOptions.WithAllowUnsafe(true));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,11 @@
<target state="new">Convert 'for' to 'foreach'</target>
<note />
</trans-unit>
<trans-unit id="Allow_unsafe_code_in_this_project">
<source>Allow unsafe code in this project</source>
<target state="new">Allow unsafe code in this project</target>
<note />
</trans-unit>
</body>
</file>
</xliff>
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,11 @@
<target state="new">Convert 'for' to 'foreach'</target>
<note />
</trans-unit>
<trans-unit id="Allow_unsafe_code_in_this_project">
<source>Allow unsafe code in this project</source>
<target state="new">Allow unsafe code in this project</target>
<note />
</trans-unit>
</body>
</file>
</xliff>
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,11 @@
<target state="new">Convert 'for' to 'foreach'</target>
<note />
</trans-unit>
<trans-unit id="Allow_unsafe_code_in_this_project">
<source>Allow unsafe code in this project</source>
<target state="new">Allow unsafe code in this project</target>
<note />
</trans-unit>
</body>
</file>
</xliff>
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,11 @@
<target state="new">Convert 'for' to 'foreach'</target>
<note />
</trans-unit>
<trans-unit id="Allow_unsafe_code_in_this_project">
<source>Allow unsafe code in this project</source>
<target state="new">Allow unsafe code in this project</target>
<note />
</trans-unit>
</body>
</file>
</xliff>
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,11 @@
<target state="new">Convert 'for' to 'foreach'</target>
<note />
</trans-unit>
<trans-unit id="Allow_unsafe_code_in_this_project">
<source>Allow unsafe code in this project</source>
<target state="new">Allow unsafe code in this project</target>
<note />
</trans-unit>
</body>
</file>
</xliff>
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,11 @@
<target state="new">Convert 'for' to 'foreach'</target>
<note />
</trans-unit>
<trans-unit id="Allow_unsafe_code_in_this_project">
<source>Allow unsafe code in this project</source>
<target state="new">Allow unsafe code in this project</target>
<note />
</trans-unit>
</body>
</file>
</xliff>
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,11 @@
<target state="new">Convert 'for' to 'foreach'</target>
<note />
</trans-unit>
<trans-unit id="Allow_unsafe_code_in_this_project">
<source>Allow unsafe code in this project</source>
<target state="new">Allow unsafe code in this project</target>
<note />
</trans-unit>
</body>
</file>
</xliff>
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,11 @@
<target state="new">Convert 'for' to 'foreach'</target>
<note />
</trans-unit>
<trans-unit id="Allow_unsafe_code_in_this_project">
<source>Allow unsafe code in this project</source>
<target state="new">Allow unsafe code in this project</target>
<note />
</trans-unit>
</body>
</file>
</xliff>
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,11 @@
<target state="new">Convert 'for' to 'foreach'</target>
<note />
</trans-unit>
<trans-unit id="Allow_unsafe_code_in_this_project">
<source>Allow unsafe code in this project</source>
<target state="new">Allow unsafe code in this project</target>
<note />
</trans-unit>
</body>
</file>
</xliff>
Loading