Skip to content

Add workarounds to IDE analyzers for missing IOperation/CFG support for using declarations#36734

Merged
mavasani merged 2 commits intodotnet:masterfrom
mavasani:UsingDeclarationsWorkarounds
Jun 25, 2019
Merged

Add workarounds to IDE analyzers for missing IOperation/CFG support for using declarations#36734
mavasani merged 2 commits intodotnet:masterfrom
mavasani:UsingDeclarationsWorkarounds

Conversation

@mavasani
Copy link
Copy Markdown
Contributor

@mavasani mavasani commented Jun 25, 2019

Workaround for #32100

  1. a08f86e: Workaround in dispose analyzer
  2. b0a9514: Workaround in unused parameter/value analyzer

@mavasani mavasani requested review from a team, dibarbet, genlu and heejaechang June 25, 2019 14:17
@mavasani mavasani added this to the 16.2.P4 milestone Jun 25, 2019
@mavasani mavasani mentioned this pull request Jun 25, 2019
Closed
@mavasani
Copy link
Copy Markdown
Contributor Author

mavasani commented Jun 25, 2019 •
edited
Loading

@ivanbasov @tmat - seems like integration tests are consistently failing in master branch for all PRs due to an NRE in EnC integration tests:

https://dev.azure.com/dnceng/public/_build/results?buildId=237952&view=ms.vss-test-web.build-test-results-tab

Server stack trace: 
   at System.Threading.Tasks.Task.ThrowIfExceptional(Boolean includeTaskCanceledExceptions)
   at System.Threading.Tasks.Task.Wait(Int32 millisecondsTimeout, CancellationToken cancellationToken)
   at System.Threading.Tasks.Task.Wait(CancellationToken cancellationToken)
   at Microsoft.VisualStudio.IntegrationTest.Utilities.InProcess.InProcComponent.InvokeOnUIThread(Action`1 action)
   at Microsoft.VisualStudio.IntegrationTest.Utilities.InProcess.TextViewWindow_InProc.ExecuteOnActiveView(Action`1 action)
   at Microsoft.VisualStudio.IntegrationTest.Utilities.InProcess.Editor_InProc.ReplaceText(String oldText, String newText)
   at System.Runtime.Remoting.Messaging.StackBuilderSink._PrivateProcessMessage(IntPtr md, Object[] args, Object server, Object[]& outArgs)
   at System.Runtime.Remoting.Messaging.StackBuilderSink.SyncProcessMessage(IMessage msg)

Exception rethrown at [0]: 
   at System.Runtime.Remoting.Proxies.RealProxy.HandleReturnMessage(IMessage reqMsg, IMessage retMsg)
   at System.Runtime.Remoting.Proxies.RealProxy.PrivateInvoke(MessageData& msgData, Int32 type)
   at Microsoft.VisualStudio.IntegrationTest.Utilities.InProcess.Editor_InProc.ReplaceText(String oldText, String newText)
   at Microsoft.VisualStudio.IntegrationTest.Utilities.OutOfProcess.Editor_OutOfProc.ReplaceText(String oldText, String newText) in /_/src/VisualStudio/IntegrationTest/TestUtilities/OutOfProcess/Editor_OutOfProc.cs:line 141
   at Roslyn.VisualStudio.IntegrationTests.VisualBasic.BasicEditAndContinue.LocalsWindowUpdatesCorrectlyDuringEnC() in /_/src/VisualStudio/IntegrationTest/IntegrationTests/VisualBasic/BasicEditAndContinue.cs:line 299
----- Inner Stack Trace -----
   at System.Threading.Tasks.Task.ThrowIfExceptional(Boolean includeTaskCanceledExceptions)
   at System.Threading.Tasks.Task.Wait(Int32 millisecondsTimeout, CancellationToken cancellationToken)
   at System.Threading.Tasks.Task.Wait(CancellationToken cancellationToken)
   at Microsoft.VisualStudio.IntegrationTest.Utilities.InProcess.InProcComponent.InvokeOnUIThread(Action`1 action)
   at Microsoft.VisualStudio.IntegrationTest.Utilities.InProcess.TextViewWindow_InProc.ExecuteOnActiveView(Action`1 action)
   at Microsoft.VisualStudio.IntegrationTest.Utilities.InProcess.TextViewWindow_InProc.PlaceCaret(String marker, Int32 charsOffset, Int32 occurrence, Boolean extendSelection, Boolean selectBlock)
   at Microsoft.VisualStudio.IntegrationTest.Utilities.InProcess.Editor_InProc.SelectText(String text)
   at Microsoft.VisualStudio.IntegrationTest.Utilities.InProcess.Editor_InProc.<>c__DisplayClass19_0.<ReplaceText>b__0(IWpfTextView view)
   at Microsoft.VisualStudio.IntegrationTest.Utilities.InProcess.TextViewWindow_InProc.<>c__DisplayClass13_0.<GetExecuteOnActionViewCallback>b__0(CancellationToken cancellationToken)
   at Microsoft.VisualStudio.IntegrationTest.Utilities.InProcess.InProcComponent.<>c__DisplayClass6_0.<<InvokeOnUIThread>b__0>d.MoveNext()
----- Inner Stack Trace -----
   at Microsoft.VisualStudio.IntegrationTest.Utilities.InProcess.TextViewWindow_InProc.<>c__DisplayClass8_0.<PlaceCaret>b__0(IWpfTextView view)
   at Microsoft.VisualStudio.IntegrationTest.Utilities.InProcess.TextViewWindow_InProc.<>c__DisplayClass13_0.<GetExecuteOnActionViewCallback>b__0(CancellationToken cancellationToken)
   at Microsoft.VisualStudio.IntegrationTest.Utilities.InProcess.InProcComponent.<>c__DisplayClass6_0.<<InvokeOnUIThread>b__0>d.MoveNext()

dibarbet
dibarbet approved these changes Jun 25, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@dibarbet dibarbet left a comment

Choose a reason for hiding this comment

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

lgtm - one minor question

src/Features/Core/Portable/DisposeAnalysis/DisposeObjectsBeforeLosingScopeDiagnosticAnalyzer.cs
ComputeDiagnostics(disposeDataAtExit, notDisposedDiagnostics, mayBeNotDisposedDiagnostics,
disposeAnalysisResult, pointsToAnalysisResult);

if (disposeAnalysisResult.ControlFlowGraph.OriginalOperation.HasAnyOperationDescendant(o => o.Kind == OperationKind.None))
Copy link
Copy Markdown
Member

@dibarbet dibarbet Jun 25, 2019

Choose a reason for hiding this comment

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

could this bail out earlier at the beginning of the try block or does this check depend on the result of ComputeDiagnostics?

Copy link
Copy Markdown
Contributor Author

@mavasani mavasani Jun 25, 2019

Choose a reason for hiding this comment

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

we can bail out earlier too. However, we will hopefully revert this soon in dev16.3 branch, which is the targeted release for #32100

@mavasani
Copy link
Copy Markdown
Contributor Author

mavasani commented Jun 25, 2019

@jinujoseph @vatsalyaagrawal for approval

@jinujoseph
Copy link
Copy Markdown
Contributor

jinujoseph commented Jun 25, 2019

@genlu could you review this as well pls

CyrusNajmabadi
CyrusNajmabadi approved these changes Jun 25, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@CyrusNajmabadi CyrusNajmabadi left a comment

Choose a reason for hiding this comment

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

Thanks for filing #32100 to track this.

Approve
Submit feedback approving these changes.

@mavasani
Copy link
Copy Markdown
Contributor Author

mavasani commented Jun 25, 2019

Thanks @genlu @CyrusNajmabadi @dibarbet

@mavasani mavasani merged commit b9b9169 into dotnet:master Jun 25, 2019
@mavasani mavasani deleted the UsingDeclarationsWorkarounds branch June 25, 2019 22:20
@mavasani mavasani mentioned this pull request Jun 28, 2019
Closed
@chsienki chsienki mentioned this pull request Oct 16, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@genlu genlu genlu approved these changes

@dibarbet dibarbet dibarbet approved these changes

@heejaechang heejaechang Awaiting requested review from heejaechang

+1 more reviewer

@CyrusNajmabadi CyrusNajmabadi CyrusNajmabadi approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Approved to merge Area-IDE

Projects

None yet

Milestone

16.2.P4

Development

Successfully merging this pull request may close these issues.

5 participants

@mavasani @jinujoseph @genlu @CyrusNajmabadi @dibarbet