Skip to content

Commit

Permalink
Track nullability in script code (#33096)
Browse files Browse the repository at this point in the history
* Analyze nullability in script initializer

* C# 8 for scripts for now

* nullable context script test

* extra null check

* added possibility to set c# Language Version on ScriptOptions

* adjust tests

* added ScriptOptionsTests for C# lang version

* adapted tests after rebase

* code review changes

* fixed test names & resx CDATA

* changed the ScriptOptions.WithLanguageVersion error message text and moved it to ScriptingResources.resx

* Update CSharpScriptingResources.Designer.cs
  • Loading branch information
filipw authored and cston committed Mar 26, 2019
1 parent 17eb073 commit 7f809ef
Show file tree
Hide file tree
Showing 28 changed files with 187 additions and 20 deletions.
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/BinderFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ internal Binder GetBinder(SyntaxNode node, CSharpSyntaxNode memberDeclarationOpt
// Unless this is interactive retrieving a binder for global statements
// at the very top-level (i.e. in a completely empty file) use
// node.Parent to maintain existing behavior.
if (!InScript || node.Kind() != SyntaxKind.CompilationUnit)
if ((!InScript || node.Kind() != SyntaxKind.CompilationUnit) && node.Parent != null)
{
node = node.Parent;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ internal static void Analyze(
DiagnosticBag diagnostics,
Action<BoundExpression, TypeWithAnnotations> callbackOpt = null)
{
if (method.IsImplicitlyDeclared && (!method.IsImplicitConstructor || method.ContainingType.IsImplicitlyDeclared))
if (method.IsImplicitlyDeclared && !method.IsImplicitConstructor && !method.IsScriptInitializer)
{
return;
}
Expand Down
8 changes: 4 additions & 4 deletions src/Scripting/CSharp/CSharpScriptCompiler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ internal sealed class CSharpScriptCompiler : ScriptCompiler
{
public static readonly ScriptCompiler Instance = new CSharpScriptCompiler();

private static readonly CSharpParseOptions s_defaultOptions = new CSharpParseOptions(kind: SourceCodeKind.Script, languageVersion: LanguageVersion.Latest);
internal static readonly CSharpParseOptions DefaultParseOptions = new CSharpParseOptions(kind: SourceCodeKind.Script, languageVersion: LanguageVersion.Latest);

private CSharpScriptCompiler()
{
Expand All @@ -23,8 +23,8 @@ private CSharpScriptCompiler()

public override bool IsCompleteSubmission(SyntaxTree tree) => SyntaxFactory.IsCompleteSubmission(tree);

public override SyntaxTree ParseSubmission(SourceText text, CancellationToken cancellationToken) =>
SyntaxFactory.ParseSyntaxTree(text, s_defaultOptions, cancellationToken: cancellationToken);
public override SyntaxTree ParseSubmission(SourceText text, ParseOptions parseOptions, CancellationToken cancellationToken) =>
SyntaxFactory.ParseSyntaxTree(text, parseOptions ?? DefaultParseOptions, cancellationToken: cancellationToken);

public override Compilation CreateSubmission(Script script)
{
Expand All @@ -40,7 +40,7 @@ public override Compilation CreateSubmission(Script script)
// TODO: report diagnostics
diagnostics.Free();

var tree = SyntaxFactory.ParseSyntaxTree(script.SourceText, s_defaultOptions, script.Options.FilePath);
var tree = SyntaxFactory.ParseSyntaxTree(script.SourceText, script.Options.ParseOptions ?? DefaultParseOptions, script.Options.FilePath);

string assemblyName, submissionTypeName;
script.Builder.GenerateSubmissionId(out assemblyName, out submissionTypeName);
Expand Down
2 changes: 1 addition & 1 deletion src/Scripting/CSharp/CSharpScriptingResources.Designer.cs

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

3 changes: 2 additions & 1 deletion src/Scripting/CSharp/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@

Microsoft.CodeAnalysis.CSharp.Scripting.ScriptOptionsExtensions
static Microsoft.CodeAnalysis.CSharp.Scripting.ScriptOptionsExtensions.WithLanguageVersion(this Microsoft.CodeAnalysis.Scripting.ScriptOptions options, Microsoft.CodeAnalysis.CSharp.LanguageVersion languageVersion) -> Microsoft.CodeAnalysis.Scripting.ScriptOptions
19 changes: 19 additions & 0 deletions src/Scripting/CSharp/ScriptOptionsExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// 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;
using Microsoft.CodeAnalysis.Scripting;

namespace Microsoft.CodeAnalysis.CSharp.Scripting
{
public static class ScriptOptionsExtensions
{
public static ScriptOptions WithLanguageVersion(this ScriptOptions options, LanguageVersion languageVersion)
{
var parseOptions = (options.ParseOptions is null)
? CSharpScriptCompiler.DefaultParseOptions
: (options.ParseOptions is CSharpParseOptions existing) ? existing : throw new InvalidOperationException(string.Format(ScriptingResources.CannotSetLanguageSpecificOption, LanguageNames.CSharp, nameof(LanguageVersion)));

return options.WithParseOptions(parseOptions.WithLanguageVersion(languageVersion));
}
}
}
36 changes: 36 additions & 0 deletions src/Scripting/CSharpTest/ScriptOptionsTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// 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;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Scripting;
using Microsoft.CodeAnalysis.Scripting;
using Microsoft.CodeAnalysis.VisualBasic;
using Roslyn.Test.Utilities;
using Xunit;

namespace Microsoft.CodeAnalysis.CSharp.Scripting.UnitTests
{
public class ScriptOptionsTests : TestBase
{
[Fact]
public void WithLanguageVersion()
{
var options = ScriptOptions.Default.WithLanguageVersion(LanguageVersion.CSharp8);
Assert.Equal(LanguageVersion.CSharp8, ((CSharpParseOptions)options.ParseOptions).LanguageVersion);
}

[Fact]
public void WithLanguageVersion_SameValueTwice_DoesNotCreateNewInstance()
{
var options = ScriptOptions.Default.WithLanguageVersion(LanguageVersion.CSharp8);
Assert.Same(options, options.WithLanguageVersion(LanguageVersion.CSharp8));
}

[Fact]
public void WithLanguageVersion_NonCSharpParseOptions_Throws()
{
var options = ScriptOptions.Default.WithParseOptions(new VisualBasicParseOptions(kind: SourceCodeKind.Script, languageVersion: VisualBasic.LanguageVersion.Latest));
Assert.Throws<InvalidOperationException>(() => options.WithLanguageVersion(LanguageVersion.CSharp8));
}
}
}
25 changes: 25 additions & 0 deletions src/Scripting/CSharpTest/ScriptTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -923,6 +923,31 @@ public void StreamWithOffset()
ScriptingTestHelpers.EvaluateScriptWithOutput(script, "Hello World!");
}

[Fact]
public void CreateScriptWithFeatureThatIsNotSupportedInTheSelectedLanguageVersion()
{
var script = CSharpScript.Create(@"string x = default;", ScriptOptions.Default.WithLanguageVersion(LanguageVersion.CSharp7));
var compilation = script.GetCompilation();

compilation.VerifyDiagnostics(
Diagnostic(ErrorCode.ERR_FeatureNotAvailableInVersion7, "default").
WithArguments("default literal", "7.1").
WithLocation(1, 12)
);
}

[Fact]
public void CreateScriptWithNullableContextWithCSharp8()
{
var script = CSharpScript.Create(@"#nullable enable
string x = null;", ScriptOptions.Default.WithLanguageVersion(LanguageVersion.CSharp8));
var compilation = script.GetCompilation();

compilation.VerifyDiagnostics(
Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(2, 28)
);
}

private class StreamOffsetResolver : SourceReferenceResolver
{
public override bool Equals(object other) => ReferenceEquals(this, other);
Expand Down
5 changes: 3 additions & 2 deletions src/Scripting/Core/Hosting/CommandLine/CommandLineRunner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,8 @@ private static ScriptOptions GetScriptOptions(CommandLineArguments arguments, st
optimizationLevel: OptimizationLevel.Debug,
allowUnsafe: true,
checkOverflow: false,
warningLevel: 4);
warningLevel: 4,
parseOptions: null);
}

internal static MetadataReferenceResolver GetMetadataReferenceResolver(CommandLineArguments arguments, TouchedFileLogger loggerOpt)
Expand Down Expand Up @@ -239,7 +240,7 @@ private void RunInteractiveLoop(ScriptOptions options, string initialScriptCodeO

input.AppendLine(line);

var tree = _scriptCompiler.ParseSubmission(SourceText.From(input.ToString()), cancellationToken);
var tree = _scriptCompiler.ParseSubmission(SourceText.From(input.ToString()), options.ParseOptions, cancellationToken);
if (_scriptCompiler.IsCompleteSubmission(tree))
{
break;
Expand Down
2 changes: 1 addition & 1 deletion src/Scripting/Core/ScriptCompiler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ internal abstract class ScriptCompiler
public abstract DiagnosticFormatter DiagnosticFormatter { get; }
public abstract StringComparer IdentifierComparer { get; }

public abstract SyntaxTree ParseSubmission(SourceText text, CancellationToken cancellationToken);
public abstract SyntaxTree ParseSubmission(SourceText text, ParseOptions parseOptions, CancellationToken cancellationToken);
public abstract bool IsCompleteSubmission(SyntaxTree tree);
}
}
15 changes: 12 additions & 3 deletions src/Scripting/Core/ScriptOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ public sealed class ScriptOptions
OptimizationLevel.Debug,
checkOverflow: false,
allowUnsafe: true,
warningLevel: 4);
warningLevel: 4,
parseOptions: null);

private static ImmutableArray<MetadataReference> GetDefaultMetadataReferences()
{
Expand Down Expand Up @@ -134,6 +135,8 @@ private static ImmutableArray<MetadataReference> GetDefaultMetadataReferences()
/// </summary>
public int WarningLevel { get; private set; }

internal ParseOptions ParseOptions { get; private set; }

internal ScriptOptions(
string filePath,
ImmutableArray<MetadataReference> references,
Expand All @@ -145,7 +148,8 @@ internal ScriptOptions(
OptimizationLevel optimizationLevel,
bool checkOverflow,
bool allowUnsafe,
int warningLevel)
int warningLevel,
ParseOptions parseOptions)
{
Debug.Assert(filePath != null);
Debug.Assert(!references.IsDefault);
Expand All @@ -164,6 +168,7 @@ internal ScriptOptions(
CheckOverflow = checkOverflow;
AllowUnsafe = allowUnsafe;
WarningLevel = warningLevel;
ParseOptions = parseOptions;
}

private ScriptOptions(ScriptOptions other)
Expand All @@ -177,7 +182,8 @@ private ScriptOptions(ScriptOptions other)
optimizationLevel: other.OptimizationLevel,
checkOverflow: other.CheckOverflow,
allowUnsafe: other.AllowUnsafe,
warningLevel: other.WarningLevel)
warningLevel: other.WarningLevel,
parseOptions: other.ParseOptions)
{
}

Expand Down Expand Up @@ -375,5 +381,8 @@ public ScriptOptions WithCheckOverflow(bool checkOverflow) =>
/// </summary>
public ScriptOptions WithWarningLevel(int warningLevel) =>
warningLevel == WarningLevel ? this : new ScriptOptions(this) { WarningLevel = warningLevel };

internal ScriptOptions WithParseOptions(ParseOptions parseOptions) =>
parseOptions == ParseOptions ? this : new ScriptOptions(this) { ParseOptions = parseOptions };
}
}
14 changes: 11 additions & 3 deletions src/Scripting/Core/ScriptingResources.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/Scripting/Core/ScriptingResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -187,4 +187,7 @@ Script directives:
<data name="AssemblyAlreadyLoadedNotSigned" xml:space="preserve">
<value>Assembly '{0}' has already been loaded from '{1}'. A different assembly with the same name can't be loaded unless it's signed: '{2}'.</value>
</data>
<data name="CannotSetLanguageSpecificOption" xml:space="preserve">
<value>Cannot set {0} specific option {1} because the options were already configured for a different language.</value>
</data>
</root>
5 changes: 5 additions & 0 deletions src/Scripting/Core/xlf/ScriptingResources.cs.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
<file datatype="xml" source-language="en" target-language="cs" original="../ScriptingResources.resx">
<body>
<trans-unit id="CannotSetLanguageSpecificOption">
<source>Cannot set {0} specific option {1} because the options were already configured for a different language.</source>
<target state="new">Cannot set {0} specific option {1} because the options were already configured for a different language.</target>
<note />
</trans-unit>
<trans-unit id="StackOverflowWhileEvaluating">
<source>!&lt;Stack overflow while evaluating object&gt;</source>
<target state="translated">!&lt;Přetečení zásobníku během vyhodnocování objektu&gt;</target>
Expand Down
5 changes: 5 additions & 0 deletions src/Scripting/Core/xlf/ScriptingResources.de.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
<file datatype="xml" source-language="en" target-language="de" original="../ScriptingResources.resx">
<body>
<trans-unit id="CannotSetLanguageSpecificOption">
<source>Cannot set {0} specific option {1} because the options were already configured for a different language.</source>
<target state="new">Cannot set {0} specific option {1} because the options were already configured for a different language.</target>
<note />
</trans-unit>
<trans-unit id="StackOverflowWhileEvaluating">
<source>!&lt;Stack overflow while evaluating object&gt;</source>
<target state="translated">!&lt;Stapelüberlauf beim Auswerten des Objekts.&gt;</target>
Expand Down
5 changes: 5 additions & 0 deletions src/Scripting/Core/xlf/ScriptingResources.es.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
<file datatype="xml" source-language="en" target-language="es" original="../ScriptingResources.resx">
<body>
<trans-unit id="CannotSetLanguageSpecificOption">
<source>Cannot set {0} specific option {1} because the options were already configured for a different language.</source>
<target state="new">Cannot set {0} specific option {1} because the options were already configured for a different language.</target>
<note />
</trans-unit>
<trans-unit id="StackOverflowWhileEvaluating">
<source>!&lt;Stack overflow while evaluating object&gt;</source>
<target state="translated">!&lt;Desbordamiento de pila al evaluar objeto&gt;</target>
Expand Down
5 changes: 5 additions & 0 deletions src/Scripting/Core/xlf/ScriptingResources.fr.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
<file datatype="xml" source-language="en" target-language="fr" original="../ScriptingResources.resx">
<body>
<trans-unit id="CannotSetLanguageSpecificOption">
<source>Cannot set {0} specific option {1} because the options were already configured for a different language.</source>
<target state="new">Cannot set {0} specific option {1} because the options were already configured for a different language.</target>
<note />
</trans-unit>
<trans-unit id="StackOverflowWhileEvaluating">
<source>!&lt;Stack overflow while evaluating object&gt;</source>
<target state="translated">!&lt;Dépassement de capacité de la pile durant l'évaluation de l'objet&gt;</target>
Expand Down
5 changes: 5 additions & 0 deletions src/Scripting/Core/xlf/ScriptingResources.it.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
<file datatype="xml" source-language="en" target-language="it" original="../ScriptingResources.resx">
<body>
<trans-unit id="CannotSetLanguageSpecificOption">
<source>Cannot set {0} specific option {1} because the options were already configured for a different language.</source>
<target state="new">Cannot set {0} specific option {1} because the options were already configured for a different language.</target>
<note />
</trans-unit>
<trans-unit id="StackOverflowWhileEvaluating">
<source>!&lt;Stack overflow while evaluating object&gt;</source>
<target state="translated">!&lt;Overflow dello stack durante la valutazione dell'oggetto&gt;</target>
Expand Down
5 changes: 5 additions & 0 deletions src/Scripting/Core/xlf/ScriptingResources.ja.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
<file datatype="xml" source-language="en" target-language="ja" original="../ScriptingResources.resx">
<body>
<trans-unit id="CannotSetLanguageSpecificOption">
<source>Cannot set {0} specific option {1} because the options were already configured for a different language.</source>
<target state="new">Cannot set {0} specific option {1} because the options were already configured for a different language.</target>
<note />
</trans-unit>
<trans-unit id="StackOverflowWhileEvaluating">
<source>!&lt;Stack overflow while evaluating object&gt;</source>
<target state="translated">!&lt;オブジェクトの評価中にスタック オーバーフローが発生しました&gt;</target>
Expand Down
Loading

0 comments on commit 7f809ef

Please sign in to comment.