Skip to content

Commit

Permalink
Merge branch 'dev/andarno/VSTHRD010Steroids' into v15.7
Browse files Browse the repository at this point in the history
  • Loading branch information
AArnott committed Feb 27, 2018
2 parents 0a03351 + a9f314d commit 9869911
Show file tree
Hide file tree
Showing 19 changed files with 383 additions and 21 deletions.
8 changes: 7 additions & 1 deletion doc/analyzers/VSTHRD010.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,13 @@ This analyzer can be configured to:
3. Recognize methods that switch to the main thread when the caller awaits them.
Calls to `JoinableTaskFactory.SwitchToMainThreadAsync` methods are pre-configured.

See our [configuration](configuration.md) topic for more information.
See our [configuration](configuration.md) topic to learn more about customizing this analyzer.

This analyzer also recognizes requirements to use the main thread transitively within your solution.
For example, if method `A()` invokes a type that we know from configuration requires the main thread,
and `B()` calls `A()`, then the `B` method also needs the UI thread transitively.
This analyzer flags `B()` as needing to call a method that throws if not already on the main thread
only when `A()` is written to call such a method.

## Examples of patterns that are flagged by this analyzer

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,121 @@ void VerifyOnUIThread() {
this.VerifyCSharpDiagnostic(test, this.expect);
}

[Fact]
public void RequiresUIThreadTransitive()
{
var test = @"
using System;
using Microsoft.VisualStudio.Shell.Interop;
class Test {
void F() {
VerifyOnUIThread();
IVsSolution sln = null;
sln.SetProperty(1000, null);
}
void G() {
F();
}
void H() {
G();
}
int MainThreadGetter {
get {
H();
return 0;
}
set {
}
}
int MainThreadSetter {
get => 0;
set => H();
}
int CallMainThreadGetter_get() => MainThreadGetter; // Flagged
int CallMainThreadGetter_get2() => this.MainThreadGetter; // Flagged
int CallMainThreadGetter_get3() => ((Test)this).MainThreadGetter; // Flagged
int CallMainThreadGetter_set() => MainThreadGetter = 1;
int CallMainThreadGetter_set2() => this.MainThreadGetter = 1;
int CallMainThreadSetter_get() => MainThreadSetter;
int CallMainThreadSetter_get2() => this.MainThreadSetter;
int CallMainThreadSetter_set() => MainThreadSetter = 1; // Flagged
int CallMainThreadSetter_set2() => this.MainThreadSetter = 1; // Flagged
int CallMainThreadSetter_set3() => ((Test)this).MainThreadSetter = 1; // Flagged
// None of these should produce diagnostics since we're not invoking the members.
string NameOfFoo1() => nameof(MainThreadGetter);
string NameOfFoo2() => nameof(MainThreadSetter);
string NameOfThisFoo1() => nameof(this.MainThreadGetter);
string NameOfThisFoo2() => nameof(this.MainThreadSetter);
string NameOfH() => nameof(H);
Action GAsDelegate() => this.G;
void VerifyOnUIThread() {
}
}
";
DiagnosticResult CreateDiagnostic(int line, int column, int endLine, int endColumn) =>
new DiagnosticResult
{
Id = this.expect.Id,
Message = this.expect.Message,
SkipVerifyMessage = this.expect.SkipVerifyMessage,
Severity = this.expect.Severity,
Locations = new[] { new DiagnosticResultLocation("Test0.cs", line, column, endLine, endColumn) },
};
var expect = new DiagnosticResult[]
{
CreateDiagnostic(12, 10, 12, 11),
CreateDiagnostic(16, 10, 16, 11),
CreateDiagnostic(21, 9, 21, 12),
CreateDiagnostic(32, 9, 32, 12),
CreateDiagnostic(35, 9, 35, 33),
CreateDiagnostic(36, 9, 36, 34),
CreateDiagnostic(37, 9, 37, 34),
CreateDiagnostic(43, 9, 43, 33),
CreateDiagnostic(44, 9, 44, 34),
CreateDiagnostic(45, 9, 45, 34),
};
this.VerifyCSharpDiagnostic(test, expect);
}

[Fact]
public void RequiresUIThreadNotTransitiveIfNotExplicit()
{
var test = @"
using System;
using Microsoft.VisualStudio.Shell.Interop;
class Test {
void F() {
IVsSolution sln = null;
sln.SetProperty(1000, null);
}
void G() {
F();
}
void H() {
G();
}
void VerifyOnUIThread() {
}
}
";
this.expect.Locations = new[] { new DiagnosticResultLocation("Test0.cs", 8, 13, 8, 24) };
this.VerifyCSharpDiagnostic(test, this.expect);
}

[Fact]
public void InvokeVsSolutionAfterSwitchedToMainThreadAsync()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,11 @@ Await JoinableTaskFactory.SwitchToMainThreadAsync() first.</source>
Await JoinableTaskFactory.SwitchToMainThreadAsync() first.</target>
<note from="MultilingualBuild" annotates="source" priority="2">{0} is a type name and {1} is the name of a method that throws if not called from the main thread.</note>
</trans-unit>
<trans-unit id="VSTHRD010_MessageFormat_TransitiveMainThreadUser" translate="yes" xml:space="preserve">
<source>Add call to {0}() at start of member body because this member invokes other members that require the main thread.</source>
<target state="new">Add call to {0}() at start of member body because this member invokes other members that require the main thread.</target>
<note from="MultilingualBuild" annotates="source" priority="2">{0} is a method name.</note>
</trans-unit>
</group>
</body>
</file>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,11 @@ Await JoinableTaskFactory.SwitchToMainThreadAsync() first.</source>
Await JoinableTaskFactory.SwitchToMainThreadAsync() first.</target>
<note from="MultilingualBuild" annotates="source" priority="2">{0} is a type name and {1} is the name of a method that throws if not called from the main thread.</note>
</trans-unit>
<trans-unit id="VSTHRD010_MessageFormat_TransitiveMainThreadUser" translate="yes" xml:space="preserve">
<source>Add call to {0}() at start of member body because this member invokes other members that require the main thread.</source>
<target state="new">Add call to {0}() at start of member body because this member invokes other members that require the main thread.</target>
<note from="MultilingualBuild" annotates="source" priority="2">{0} is a method name.</note>
</trans-unit>
</group>
</body>
</file>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,11 @@ Await JoinableTaskFactory.SwitchToMainThreadAsync() first.</source>
Await JoinableTaskFactory.SwitchToMainThreadAsync() first.</target>
<note from="MultilingualBuild" annotates="source" priority="2">{0} is a type name and {1} is the name of a method that throws if not called from the main thread.</note>
</trans-unit>
<trans-unit id="VSTHRD010_MessageFormat_TransitiveMainThreadUser" translate="yes" xml:space="preserve">
<source>Add call to {0}() at start of member body because this member invokes other members that require the main thread.</source>
<target state="new">Add call to {0}() at start of member body because this member invokes other members that require the main thread.</target>
<note from="MultilingualBuild" annotates="source" priority="2">{0} is a method name.</note>
</trans-unit>
</group>
</body>
</file>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,11 @@ Await JoinableTaskFactory.SwitchToMainThreadAsync() first.</source>
Await JoinableTaskFactory.SwitchToMainThreadAsync() first.</target>
<note from="MultilingualBuild" annotates="source" priority="2">{0} is a type name and {1} is the name of a method that throws if not called from the main thread.</note>
</trans-unit>
<trans-unit id="VSTHRD010_MessageFormat_TransitiveMainThreadUser" translate="yes" xml:space="preserve">
<source>Add call to {0}() at start of member body because this member invokes other members that require the main thread.</source>
<target state="new">Add call to {0}() at start of member body because this member invokes other members that require the main thread.</target>
<note from="MultilingualBuild" annotates="source" priority="2">{0} is a method name.</note>
</trans-unit>
</group>
</body>
</file>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,11 @@ Await JoinableTaskFactory.SwitchToMainThreadAsync() first.</source>
Await JoinableTaskFactory.SwitchToMainThreadAsync() first.</target>
<note from="MultilingualBuild" annotates="source" priority="2">{0} is a type name and {1} is the name of a method that throws if not called from the main thread.</note>
</trans-unit>
<trans-unit id="VSTHRD010_MessageFormat_TransitiveMainThreadUser" translate="yes" xml:space="preserve">
<source>Add call to {0}() at start of member body because this member invokes other members that require the main thread.</source>
<target state="new">Add call to {0}() at start of member body because this member invokes other members that require the main thread.</target>
<note from="MultilingualBuild" annotates="source" priority="2">{0} is a method name.</note>
</trans-unit>
</group>
</body>
</file>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,11 @@ Await JoinableTaskFactory.SwitchToMainThreadAsync() first.</source>
Await JoinableTaskFactory.SwitchToMainThreadAsync() first.</target>
<note from="MultilingualBuild" annotates="source" priority="2">{0} is a type name and {1} is the name of a method that throws if not called from the main thread.</note>
</trans-unit>
<trans-unit id="VSTHRD010_MessageFormat_TransitiveMainThreadUser" translate="yes" xml:space="preserve">
<source>Add call to {0}() at start of member body because this member invokes other members that require the main thread.</source>
<target state="new">Add call to {0}() at start of member body because this member invokes other members that require the main thread.</target>
<note from="MultilingualBuild" annotates="source" priority="2">{0} is a method name.</note>
</trans-unit>
</group>
</body>
</file>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,11 @@ Await JoinableTaskFactory.SwitchToMainThreadAsync() first.</source>
Await JoinableTaskFactory.SwitchToMainThreadAsync() first.</target>
<note from="MultilingualBuild" annotates="source" priority="2">{0} is a type name and {1} is the name of a method that throws if not called from the main thread.</note>
</trans-unit>
<trans-unit id="VSTHRD010_MessageFormat_TransitiveMainThreadUser" translate="yes" xml:space="preserve">
<source>Add call to {0}() at start of member body because this member invokes other members that require the main thread.</source>
<target state="new">Add call to {0}() at start of member body because this member invokes other members that require the main thread.</target>
<note from="MultilingualBuild" annotates="source" priority="2">{0} is a method name.</note>
</trans-unit>
</group>
</body>
</file>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,11 @@ Await JoinableTaskFactory.SwitchToMainThreadAsync() first.</source>
Await JoinableTaskFactory.SwitchToMainThreadAsync() first.</target>
<note from="MultilingualBuild" annotates="source" priority="2">{0} is a type name and {1} is the name of a method that throws if not called from the main thread.</note>
</trans-unit>
<trans-unit id="VSTHRD010_MessageFormat_TransitiveMainThreadUser" translate="yes" xml:space="preserve">
<source>Add call to {0}() at start of member body because this member invokes other members that require the main thread.</source>
<target state="new">Add call to {0}() at start of member body because this member invokes other members that require the main thread.</target>
<note from="MultilingualBuild" annotates="source" priority="2">{0} is a method name.</note>
</trans-unit>
</group>
</body>
</file>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,11 @@ Await JoinableTaskFactory.SwitchToMainThreadAsync() first.</source>
Await JoinableTaskFactory.SwitchToMainThreadAsync() first.</target>
<note from="MultilingualBuild" annotates="source" priority="2">{0} is a type name and {1} is the name of a method that throws if not called from the main thread.</note>
</trans-unit>
<trans-unit id="VSTHRD010_MessageFormat_TransitiveMainThreadUser" translate="yes" xml:space="preserve">
<source>Add call to {0}() at start of member body because this member invokes other members that require the main thread.</source>
<target state="new">Add call to {0}() at start of member body because this member invokes other members that require the main thread.</target>
<note from="MultilingualBuild" annotates="source" priority="2">{0} is a method name.</note>
</trans-unit>
</group>
</body>
</file>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,11 @@ Await JoinableTaskFactory.SwitchToMainThreadAsync() first.</source>
Await JoinableTaskFactory.SwitchToMainThreadAsync() first.</target>
<note from="MultilingualBuild" annotates="source" priority="2">{0} is a type name and {1} is the name of a method that throws if not called from the main thread.</note>
</trans-unit>
<trans-unit id="VSTHRD010_MessageFormat_TransitiveMainThreadUser" translate="yes" xml:space="preserve">
<source>Add call to {0}() at start of member body because this member invokes other members that require the main thread.</source>
<target state="new">Add call to {0}() at start of member body because this member invokes other members that require the main thread.</target>
<note from="MultilingualBuild" annotates="source" priority="2">{0} is a method name.</note>
</trans-unit>
</group>
</body>
</file>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,11 @@ Await JoinableTaskFactory.SwitchToMainThreadAsync() first.</source>
Await JoinableTaskFactory.SwitchToMainThreadAsync() first.</target>
<note from="MultilingualBuild" annotates="source" priority="2">{0} is a type name and {1} is the name of a method that throws if not called from the main thread.</note>
</trans-unit>
<trans-unit id="VSTHRD010_MessageFormat_TransitiveMainThreadUser" translate="yes" xml:space="preserve">
<source>Add call to {0}() at start of member body because this member invokes other members that require the main thread.</source>
<target state="new">Add call to {0}() at start of member body because this member invokes other members that require the main thread.</target>
<note from="MultilingualBuild" annotates="source" priority="2">{0} is a method name.</note>
</trans-unit>
</group>
</body>
</file>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,11 @@ Await JoinableTaskFactory.SwitchToMainThreadAsync() first.</source>
Await JoinableTaskFactory.SwitchToMainThreadAsync() first.</target>
<note from="MultilingualBuild" annotates="source" priority="2">{0} is a type name and {1} is the name of a method that throws if not called from the main thread.</note>
</trans-unit>
<trans-unit id="VSTHRD010_MessageFormat_TransitiveMainThreadUser" translate="yes" xml:space="preserve">
<source>Add call to {0}() at start of member body because this member invokes other members that require the main thread.</source>
<target state="new">Add call to {0}() at start of member body because this member invokes other members that require the main thread.</target>
<note from="MultilingualBuild" annotates="source" priority="2">{0} is a method name.</note>
</trans-unit>
</group>
</body>
</file>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,11 @@ Await JoinableTaskFactory.SwitchToMainThreadAsync() first.</source>
Await JoinableTaskFactory.SwitchToMainThreadAsync() first.</target>
<note from="MultilingualBuild" annotates="source" priority="2">{0} is a type name and {1} is the name of a method that throws if not called from the main thread.</note>
</trans-unit>
<trans-unit id="VSTHRD010_MessageFormat_TransitiveMainThreadUser" translate="yes" xml:space="preserve">
<source>Add call to {0}() at start of member body because this member invokes other members that require the main thread.</source>
<target state="new">Add call to {0}() at start of member body because this member invokes other members that require the main thread.</target>
<note from="MultilingualBuild" annotates="source" priority="2">{0} is a method name.</note>
</trans-unit>
</group>
</body>
</file>
Expand Down

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

4 changes: 4 additions & 0 deletions src/Microsoft.VisualStudio.Threading.Analyzers/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -257,4 +257,8 @@ Use AsyncLazy&lt;T&gt; instead.</value>
Await JoinableTaskFactory.SwitchToMainThreadAsync() first.</value>
<comment>{0} is a type name and {1} is the name of a method that throws if not called from the main thread.</comment>
</data>
<data name="VSTHRD010_MessageFormat_TransitiveMainThreadUser" xml:space="preserve">
<value>Add call to {0}() at start of member body because this member invokes other members that require the main thread.</value>
<comment>{0} is a method name.</comment>
</data>
</root>
20 changes: 18 additions & 2 deletions src/Microsoft.VisualStudio.Threading.Analyzers/Utils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,22 @@ internal static bool IsObsolete(this ISymbol symbol)
return symbol.GetAttributes().Any(a => a.AttributeClass.Name == nameof(ObsoleteAttribute) && a.AttributeClass.BelongsToNamespace(Namespaces.System));
}

internal static bool IsOnLeftHandOfAssignment(SyntaxNode syntaxNode)
{
SyntaxNode parent = null;
while ((parent = syntaxNode.Parent) != null)
{
if (parent is AssignmentExpressionSyntax assignment)
{
return assignment.Left == syntaxNode;
}

syntaxNode = parent;
}

return false;
}

internal static IEnumerable<ITypeSymbol> FindInterfacesImplemented(this ISymbol symbol)
{
if (symbol == null)
Expand Down Expand Up @@ -607,9 +623,9 @@ internal static NameSyntax QualifyName(IReadOnlyList<string> qualifiers, SimpleN
/// <summary>
/// Determines whether an expression appears inside a C# "nameof" pseudo-method.
/// </summary>
internal static bool IsWithinNameOf(ExpressionSyntax memberAccess)
internal static bool IsWithinNameOf(SyntaxNode syntaxNode)
{
var invocation = memberAccess?.FirstAncestorOrSelf<InvocationExpressionSyntax>();
var invocation = syntaxNode?.FirstAncestorOrSelf<InvocationExpressionSyntax>();
return (invocation?.Expression as IdentifierNameSyntax)?.Identifier.Text == "nameof"
&& invocation.ArgumentList.Arguments.Count == 1;
}
Expand Down
Loading

0 comments on commit 9869911

Please sign in to comment.