These are chat archives for DotNetAnalyzers/StyleCopAnalyzers

1st
Dec 2015
Dennis Fischer
@pdelvo
Dec 01 2015 00:00
if they use binding redirects we should get them for free
John Venable
@Noryoko
Dec 01 2015 00:00
I'm in favor of pushing the code fix to 1.1 and shoring up some of the other issues. Such as having to run it multiple times.
Sam Harwell
@sharwell
Dec 01 2015 00:02
@Noryoko so we disable the ordering code fix altogether for 1.0, or we just disable it when the documented contains preprocessor directives?
@pdelvo I'm going to create an experimental branch where I update to roslyn 1.1 and run the tests there, just to make sure nothing breaks.
Dennis Fischer
@pdelvo
Dec 01 2015 00:03
three tests fail. two of those fails were caused by a bug fix where we just have to change the tests. and one fail where I actually dont know how this ever worked. I have not really looked into it.
John Venable
@Noryoko
Dec 01 2015 00:04
in the current PR it's close to the second option. I've been working on addressing the feedback
Dennis Fischer
@pdelvo
Dec 01 2015 00:04
pdelvo/StyleCopAnalyzers@eb47d62
John Venable
@Noryoko
Dec 01 2015 00:04
The first option depends on if needing to run it multiple times is a deal breaker
Dennis Fischer
@pdelvo
Dec 01 2015 00:05
Adding stylecopanalyzers to a project fixing the ordering rules is the most annoying thing
John Venable
@Noryoko
Dec 01 2015 00:16
I'm leaning towards disabling all together for a few reasons. 1 It's very annoying to use if you have multiple violations in a single document. 2 It can break things. 3 The preferred order matches classic.
Sam Harwell
@sharwell
Dec 01 2015 00:21
Yeah, it would be good to fix #1767 before we enable it again call it "stable"
John Venable
@Noryoko
Dec 01 2015 00:22
Yeah, I would rather get it right than get it close
I believe there are 3 open issues regarding the fix currently get those addressed any any others that come up between now and then
Sam Harwell
@sharwell
Dec 01 2015 00:29
@Noryoko I assigned #1868 to you
John Venable
@Noryoko
Dec 01 2015 00:31
how do you want to handle all the code fix unit tests?
Sam Harwell
@sharwell
Dec 01 2015 00:32
They don't rely on the attribute; none of them will break
Correction. One of them might break (the one that verifies that the attribute exists)
Let me know which ones break if you aren't sure how to address it
Sam Harwell
@sharwell
Dec 01 2015 00:40
@pdelvo Things are looking good. The only test failures following the 1.1 update are non-breaking
(Some places where duplicate diagnostics were reported no longer report diagnostics)
On the negative side, one of the analyzers provided by Microsoft to help analyzer authors is giving an AD0001
Sam Harwell
@sharwell
Dec 01 2015 00:51
Looks like Roslyn 1.1 loads a solution ~8.7% faster
John Venable
@Noryoko
Dec 01 2015 00:51
that's a bit of an improvement
Johan Larsson
@JohanLarsson
Dec 01 2015 00:58
heard there will be a c# repl in vs2015 update 1
Sam Harwell
@sharwell
Dec 01 2015 00:59
With 1.1 running on Roslyn.sln, I see the following compared to 1.0 running on the same Roslyn.sln:
  • 4 more instances of SA1101 (0.0036%)
  • 26 fewer instances of SA1124 (2.1259%)
  • 1 more instance of SA1130 (2.0408%)
  • 120 fewer instances of SA1201 (2.8867%)
  • 6 fewer instances of SA1202 (0.1340%)
  • 47 fewer instances of SA1204 (5.1819%)
  • 29 fewer instances of SA1516 (0.8547%)
I also observe 1.1 at ~23.4% slower than 1.0 at running our analyzers on Roslyn.sln.
@mavasani you'll be interested in these stats :arrow_up:
Yaakov
@yaakov-h
Dec 01 2015 01:01
1.1 = VS2015 Update 1?
John Venable
@Noryoko
Dec 01 2015 01:02
Only 1 failed test. The one that checks for the attribute
Sam Harwell
@sharwell
Dec 01 2015 01:02
@yaakov-h We build directly against Microsoft.CodeAnalysis (the compiler library). Visual Studio 2015 Update 1 includes version 1.1, but we are independent of it. For example, you can run StyleCopTester in Visual Studio 2015 RTM using the 1.1 libraries, and you can run StyleCopTester in Visual Studio 2015 Update 1 using the 1.0 libraries.
Yaakov
@yaakov-h
Dec 01 2015 01:02
oh, TIL. I thought the Roslyn side was provided by VS
Sam Harwell
@sharwell
Dec 01 2015 01:05
@Noryoko Add the [NoCodeFix] attribute to the code fix provider, and update the test to ignore code fix providers that have that attribute
Johan Larsson
@JohanLarsson
Dec 01 2015 01:05
any rumors about when c#7 is ready?
I want that sweet record syntax
Sam Harwell
@sharwell
Dec 01 2015 01:05
Also update the test to assert that code fix providers which have [NoCodeFix] do not also have [ExportCodeFixProvider]
@JohanLarsson Probably won't be for a while
Johan Larsson
@JohanLarsson
Dec 01 2015 01:07
yeah, if they aim for solving the null problem it will probably take a while
hopefully things will be faster now when roslyn is shipped
Sam Harwell
@sharwell
Dec 01 2015 01:09
@Noryoko I'll be back later this evening in case you get that PR done
John Venable
@Noryoko
Dec 01 2015 01:09
should be done shortly
I'll be around if you have any feedback
Sam Harwell
@sharwell
Dec 01 2015 01:10
good to hear :)
Sam Harwell
@sharwell
Dec 01 2015 01:23
Out for ~90 mintues for dinner
Yaakov
@yaakov-h
Dec 01 2015 01:23
enjoy
John Venable
@Noryoko
Dec 01 2015 01:24
You'll have a PR waiting when you get back
Sam Harwell
@sharwell
Dec 01 2015 03:11
I see that!
John Venable
@Noryoko
Dec 01 2015 03:14
Well that was quick
Sam Harwell
@sharwell
Dec 01 2015 03:18
Now just waiting for the CI builds to complete
Nope, forgot something
Sam Harwell
@sharwell
Dec 01 2015 03:27
#1873
Make sure the two remaining rules with no implementation don't show up in the rule set editor
back in ~30 minutes
Sam Harwell
@sharwell
Dec 01 2015 06:12
@/all As of right now, we have a stabilization branch for the 1.0.0 release. Work on master from here forward will apply to the post-1.0.0 release. Pull requests sent to stabilization are only for bug fixes which have an associated issue, and require two code reviews from active contributors to accept. The 1.0.0 (stable) release will ship after 30 days without a commit to the stabilization branch.
Oh, and I released 1.0.0 RC 1. If you want to see more users involved with final testing, consider retweeting the announcement:
https://twitter.com/samharwell/status/671571530199904256
Dmitry Turin
@Maxwe11
Dec 01 2015 11:55
nice
Olly Atkins
@oatkins
Dec 01 2015 16:03
Is there any way of running code fixes at the command line (and preferably in a 64-bit process)? If I try and batch fix my solutions, I get OOM after memory usage hits about 2 Gb.
Nikolay Kostov
@NikolayIT
Dec 01 2015 17:02
@oatkins, is your problem similar to DotNetAnalyzers/StyleCopAnalyzers#1877 ?
Dennis Fischer
@pdelvo
Dec 01 2015 17:22
If this is a one time thing you could use StyleCopTester. it applies fixes in memory. To write them to disk you just need to apply the changes to the workspace at the end. It might be usefull to create a command line switch for it.
Olly Atkins
@oatkins
Dec 01 2015 18:07
@NikolayIT It does look similar, though I had similar behaviour on VS 2015 RTM (not Update 1) too.
Nikolay Kostov
@NikolayIT
Dec 01 2015 18:24
@sharwell do you consider upgrading to Microsoft.CodeAnalysis.* 1.1.0 before next RC version of StyleCopAnalyzers?
Sam Harwell
@sharwell
Dec 01 2015 18:42
@NikolayIT It doesn't seem to offer us anything
Nikolay Kostov
@NikolayIT
Dec 01 2015 18:50
Not even performance improvements? Just for you information: I've upgraded it locally and everything compiled successfully and all unit tests passed.
Sam Harwell
@sharwell
Dec 01 2015 18:51
There is an assembly binding redirect in 1.1. Our analyzers run with 1.1 in Update 1 even if we build them against 1.0.
And no, performance is ~23% worse by my measurements (we're working on more accurate tests to confirm this)
Nikolay Kostov
@NikolayIT
Dec 01 2015 18:52
Got it. Then sticking with 1.0.0 seems better
Sam Harwell
@sharwell
Dec 01 2015 18:53
Sticking with 1.0 means users who haven't upgraded to Visual Studio 2015 Update 1 can still use our analyzers
Nikolay Kostov
@NikolayIT
Dec 01 2015 18:53
:+1:
Martin Costello
@martincostello
Dec 01 2015 20:25

I'm just having a go at implementing SA1629 at the moment, and I'm getting behaviour in my tests that I don't understand.

In some situations the line reported for XML comments lacking a period is the line declaring the member (e.g. public void Foo(int bar)) rather that the line where the comment is. I used the following construct as a basis for the code from some other rules, and it's working in some situations so I'm a bit confused: context.ReportDiagnostic(Diagnostic.Create(Descriptor, element.GetLocation()));.

Calling GetLineSpan() on the Location in the immediate window gives the right line number. Sorry if the answer is something obvious, but this is my first go at implementing an analyser :)

Vincent Weijsters
@vweijsters
Dec 01 2015 20:35
@sharwell I'm going to have a look at #1875 and #1878 tonight. If I can find the issue, I'll submit a PR on stabilization
Nikolay Kostov
@NikolayIT
Dec 01 2015 20:37
@vweijsters, #1878 is already in progress. You can take a look at the PR. Also please take a look at my last comment there.
Vincent Weijsters
@vweijsters
Dec 01 2015 20:37
ok, thanks. I overlooked that one.
Vincent Weijsters
@vweijsters
Dec 01 2015 21:24
@martincostello I would like to help you, but there is not enough information for me to say anything informed. Could you share the code of the analyzer on gist?