VSDiagnostics

Hosch250

12:02 AM

Yep.

Duga

[Vannevelj/VSDiagnostics] Hosch250 pushed commit 8526244a to develop

Update README.md

Add Switch Does Not Handle All Enum Members and Equals and GetHashcode Not Implemented Together

Hosch250

@JeroenVannevel There you go.

Duga

[Vannevelj/VSDiagnostics] Hosch250 [commented on VSDiagnostics/VSDiagnostics/VSDiagnostics/Diagnostics/General/EqualsAndGetHashc‌odeNotImplementedTogether/EqualsAndGetHashcodeNotImplementedTogetherAnalyzer.cs]‌(github.com/Vannevelj/VSDiagnostics/pull/…) of pull request #505: Close #172

[Vannevelj/VSDiagnostics] build for commit 8526244a on develop: AppVeyor build succeeded

[Vannevelj/VSDiagnostics] Hosch250 commented on pull request #505: Close #172

> I'll update the Readme when it gets merged. I think there are a couple others missing as well.

Hosch250

@Duga Actually, I killed that comment.

@JeroenVannevel I updated the readme in Develop, but it didn't seem to get attached to the PR.

Jeroen Vannevel

yeah, I just noticed

you have to change the readme in the PR's branch

Hosch250

12:14 AM

I did.

Jeroen Vannevel

you pushed it to my develop

for some reason

Hosch250

Oh, apparently I didn't.

Oh, huh.

Sorry about that.

Jeroen Vannevel

I didn't even know you could do that

maybe because of collaborator status

yeah np

Hosch250

Yeah, I can. You can change that.

Duga

[Vannevelj/VSDiagnostics] Vannevelj merged pull request #505: Close #172 from Hosch250/VSDiagnostics/develop into Vannevelj/VSDiagnostics/develop

[Vannevelj/VSDiagnostics] Vannevelj closed issue #172: Implement EqualsAndGetHashcodeNotImplementedTogether

[Vannevelj/VSDiagnostics] Vannevelj pushed commit 596370b9 to develop

Close #172 (#505)

* Push so I can check something else out.

* Display error message when no branches are found.

* Code fix works.

* Remove try/catch used in debugging.

* Fix Equal()'s implementation, add a couple tests.

* Respond to comments.

* Fix bug when base class implements both but derived class only implements one.

* Fix conflicting ID's

* Fix NRE

Hosch250

12:17 AM

@Duga Oooh, nice update.

Duga

[Hosch250/VSDiagnostics] Hosch250 pushed commit 8526244a to develop

Update README.md

Add Switch Does Not Handle All Enum Members and Equals and GetHashcode Not Implemented Together

[Hosch250/VSDiagnostics] Vannevelj pushed commit 596370b9 to develop

Close #172 (#505)

* Push so I can check something else out.

* Display error message when no branches are found.

* Code fix works.

* Remove try/catch used in debugging.

* Fix Equal()'s implementation, add a couple tests.

* Respond to comments.

* Fix bug when base class implements both but derived class only implements one.

* Fix conflicting ID's

* Fix NRE

[Hosch250/VSDiagnostics] Hosch250 pushed commit 20a2a28b to develop: Merge branch 'develop' of github.com/Vannevelj/VSDiagnostics into develop

Hosch250

Oh, is that because of the Squash feature?

Duga

[Vannevelj/VSDiagnostics] Vannevelj added label priority - low to issue #539: Add self-mutating struct test that assigns to default

[Vannevelj/VSDiagnostics] Vannevelj opened issue #539: Add self-mutating struct test that assigns to default

> I think it's already covered but I'd like an explicit test that covers `this = default(MyStruct)` for `StructDoesNotMutateSelf`.

Also: there's a test with some bad indentation -- fix that

[Vannevelj/VSDiagnostics] Vannevelj added label type - task to issue #539: Add self-mutating struct test that assigns to default

Jeroen Vannevel

@Hosch250 yeah. It combines the individual commit messages into the squashed commit

Hosch250

Yeah, I know.

RubberDuck was helping me with that once, but it never worked after I committed to GitHub.

Duga

12:22 AM

[Vannevelj/VSDiagnostics] build for commit 596370b9 on develop: AppVeyor build succeeded

Hosch250

1:46 AM

@JeroenVannevel The latest analyzer fires when the methods do not have a body.

The fix in this case results in this:

    public class Foo1 : MyBaseClass
    {
        public override int GetHashCode()
public override bool Equals(object obj)
        {
            throw new System.NotImplementedException();
        }
    }

I'm guessing that is fine, but just checking.

Duga

[Hosch250/VSDiagnostics] Hosch250 pushed commit 574d2a57 to develop: Adjust underlines to only highlight class name.

[Hosch250/VSDiagnostics] Hosch250 deleted branch develop

[Hosch250/VSDiagnostics] Hosch250 created branch develop

[Hosch250/VSDiagnostics] Hosch250 deleted branch develop

[Hosch250/VSDiagnostics] Hosch250 created branch develop

[Hosch250/VSDiagnostics] Vannevelj pushed commit c93cc759 to develop

Make SwitchDoesNotHandleAllEnumOptions show lines on the expression. (#531)

* Make SwitchDoesNotHandleAllEnumOptions show lines on the expression.

* Fix cast in code fix.

[Hosch250/VSDiagnostics] Vannevelj pushed commit d7ec1215 to develop

Castingissues (#534)

* prepared tests

* Fixed some tests and solved all issues

[Hosch250/VSDiagnostics] Vannevelj pushed commit 6b018ac4 to develop: Implemented #236 (#536)

[Hosch250/VSDiagnostics] Hosch250 pushed commit 8526244a to develop

Update README.md

Add Switch Does Not Handle All Enum Members and Equals and GetHashcode Not Implemented Together

[Hosch250/VSDiagnostics] Vannevelj pushed commit 596370b9 to develop

Close #172 (#505)

* Push so I can check something else out.

* Display error message when no branches are found.

* Code fix works.

* Remove try/catch used in debugging.

* Fix Equal()'s implementation, add a couple tests.

* Respond to comments.

* Fix bug when base class implements both but derived class only implements one.

* Fix conflicting ID's

* Fix NRE

[Hosch250/VSDiagnostics] Hosch250 pushed commit 59118b9b to develop: Only underline class name.

[Vannevelj/VSDiagnostics] Hosch250 created pull request #540: Only underline class name. to merge Hosch250/VSDiagnostics/develop into Vannevelj/VSDiagnostics/develop

Hosch250

That fixes the mega-highlight ^

Duga

2:03 AM

[Vannevelj/VSDiagnostics] build for commit 59118b9b on unknown branch: AppVeyor build succeeded

Hosch250

@JeroenVannevel RoslynTester doesn't test the range an analyzer is available in, does it?

Roslyn does. In C#, it is signaled with [|code here|].

Would that be something you would be interested in?

Jeroen Vannevel

8:11 AM

@Hosch250 There is an overload for that -- you can pass in a Location

Duga

[Vannevelj/VSDiagnostics] Vannevelj merged pull request #540: Only underline class name. from Hosch250/VSDiagnostics/develop into Vannevelj/VSDiagnostics/develop

[Vannevelj/VSDiagnostics] Vannevelj pushed commit 03765c90 to develop: Only underline class name. (#540)

[Vannevelj/VSDiagnostics] build for commit 03765c90 on develop: AppVeyor build succeeded

Jeroen Vannevel

10:17 AM

@Hosch250 yeah that's fine

Hosch250

2:46 PM

OK.

Duga

4:15 PM

[Vannevelj/VSDiagnostics] Hosch250 synchronized pull request #510: Close #3

[Hosch250/VSDiagnostics] Hosch250 pushed 15 commits to Feature3

[Hosch250/VSDiagnostics] Vannevelj pushed commit 03765c90 to Feature3: Only underline class name. (#540)

[Hosch250/VSDiagnostics] Hosch250 pushed commit c0c5c339 to Feature3: Pull from develop

[Vannevelj/VSDiagnostics] Hosch250 synchronized pull request #510: Close #3

[Hosch250/VSDiagnostics] Hosch250 pushed commit 48df8432 to Feature3: Adjust conflicting ID's caused by merge conflicts.

[Vannevelj/VSDiagnostics] Hosch250 synchronized pull request #510: Close #3

[Hosch250/VSDiagnostics] Hosch250 pushed commit 3a09599d to Feature3: Update README.md

[Vannevelj/VSDiagnostics] build for commit c0c5c339 on unknown branch: AppVeyor build succeeded

[Vannevelj/VSDiagnostics] Hosch250 synchronized pull request #510: Close #3

[Hosch250/VSDiagnostics] Hosch250 pushed commit efd6b0ca to Feature3: Respond to comments.

[Hosch250/VSDiagnostics] Hosch250 pushed commit 540bbf22 to Feature3: Merge branch 'Feature3' of github.com/Hosch250/VSDiagnostics.git

[Vannevelj/VSDiagnostics] build for commit 48df8432 on unknown branch: AppVeyor build succeeded

[Vannevelj/VSDiagnostics] build for commit 3a09599d on unknown branch: AppVeyor build succeeded

[Vannevelj/VSDiagnostics] build for commit 540bbf22 on unknown branch: AppVeyor build succeeded

[Vannevelj/VSDiagnostics] Hosch250 synchronized pull request #532: Remove Linq from Analyzers

[Hosch250/VSDiagnostics] Hosch250 pushed commit e7a4b638 to Issue493: Address comments.

[Vannevelj/VSDiagnostics] Hosch250 synchronized pull request #532: Remove Linq from Analyzers

[Hosch250/VSDiagnostics] Hosch250 pushed 6 commits to Issue493

[Hosch250/VSDiagnostics] Hosch250 pushed commit 82ad3355 to Issue493: Merge branch 'develop' of github.com/Vannevelj/VSDiagnostics into Issue493

[Vannevelj/VSDiagnostics] build for commit e7a4b638 on unknown branch: AppVeyor build succeeded

[Vannevelj/VSDiagnostics] build for commit 82ad3355 on unknown branch: AppVeyor build succeeded

[Vannevelj/VSDiagnostics] Hosch250 synchronized pull request #532: Remove Linq from Analyzers

[Hosch250/VSDiagnostics] Hosch250 pushed commit e69e1e51 to Issue493: Remove Linq from extensions.

Hosch250

4:59 PM

@JeroenVannevel I think those are both ready for merging. Check them first, though.

Duga

[Vannevelj/VSDiagnostics] build for commit e69e1e51 on unknown branch: AppVeyor build succeeded

Duga

9:29 PM

[Vannevelj/VSDiagnostics] Vannevelj created branch redundantprivatesetter

[Vannevelj/VSDiagnostics] Vannevelj pushed commit e9abb1f5 to redundantprivatesetter: Initial tests & first failsafe checks in analyzer

[Vannevelj/VSDiagnostics] build for commit e9abb1f5 on redundantprivatesetter: AppVeyor build failed

BUILD FAILURE!

[Vannevelj/VSDiagnostics] Vannevelj pushed commit bbc0c48e to develop

Close #3 (#510)

* Basic initial implementation.

* Ignore interfaces.

* Test for multiple generic arguments (as in Dictionary<T, T>)

* Another test.

* Address comments and clean up extensions.

* Delete unused extension.

* Fix failing tests.

* Adjust conflicting ID's caused by merge conflicts.

* Update README.md

* Respond to comments.

[Vannevelj/VSDiagnostics] Vannevelj closed issue #3: Implement ElementaryMethodsOfTypeInCollectionNotOverridden

[Vannevelj/VSDiagnostics] Vannevelj merged pull request #510: Close #3 from Hosch250/VSDiagnostics/Feature3 into Vannevelj/VSDiagnostics/develop

[Vannevelj/VSDiagnostics] Vannevelj commented on VSDiagnostics/VSDiagnostics/VSDiagnostics/Utilities/Extensions.cs of pull request #532: Remove Linq from Analyzers

[Vannevelj/VSDiagnostics] build for commit bbc0c48e on develop: AppVeyor build succeeded

[Vannevelj/VSDiagnostics] Vannevelj commented on VSDiagnostics/VSDiagnostics/VSDiagnostics/Utilities/Extensions.cs of pull request #532: Remove Linq from Analyzers

Jeroen Vannevel

9:49 PM

@Hosch250 merge conflict

Hosch250

Ugh.

I'll pull from my end.

Probably because you merge the one.

Jeroen Vannevel

yup

Duga

[Vannevelj/VSDiagnostics] Hosch250 commented on VSDiagnostics/VSDiagnostics/VSDiagnostics/Utilities/Extensions.cs of pull request #532: Remove Linq from Analyzers

[Vannevelj/VSDiagnostics] Hosch250 synchronized pull request #532: Remove Linq from Analyzers

Hosch250

Don't merge yet. I might have broken something when resolving the conflicts.

Darn.

Duga

[Vannevelj/VSDiagnostics] build for commit 8b0207be on unknown branch: AppVeyor build failed

BUILD FAILURE!

Duga

10:17 PM

[Hosch250/VSDiagnostics] Hosch250 pushed commit e90a800d to Issue493: Fix bugs caused by merge conflicts.

Hosch250

OK, this should work.

Duga

[Vannevelj/VSDiagnostics] Hosch250 synchronized pull request #532: Remove Linq from Analyzers

[Vannevelj/VSDiagnostics] build for commit e90a800d on unknown branch: AppVeyor build succeeded

[Hosch250/VSDiagnostics] Vannevelj commented on VSDiagnostics/VSDiagnostics/VSDiagnostics/Diagnostics/Attributes/OnPropertyChang‌edWithoutCallerMemberName/OnPropertyChangedWithoutCallerMemberNameAnalyzer.cs of commit...

e90a800d

> Is this one using the wrong overload now?

Hosch250

10:49 PM

Looks like it.

That is where one of the conflicts was.

Duga

[Vannevelj/VSDiagnostics] Hosch250 synchronized pull request #532: Remove Linq from Analyzers

[Hosch250/VSDiagnostics] Hosch250 pushed commit 3b91b75a to Issue493: Use custom OfType.

[Vannevelj/VSDiagnostics] Hosch250 edited pull request #532: Remove Linq from Analyzers

Hosch250

Apparently, that SymbolKind.Method triggering on properties is the correct behavior.

MethodSymbol symbolizes all "method-like" symbols, as I mentioned on GitHub, including properties and ctors.

Jeroen Vannevel

yeah I figured it is

but we might make wrong assumptions from it

like registering an analyzer on SymbolKind.Method and then casting to MethodDeclarationSyntax

or something like that

It's worth verifying we don't do that anywhere

Duga

[Vannevelj/VSDiagnostics] build for commit 3b91b75a on unknown branch: AppVeyor build succeeded

Hosch250

Right. We need to check it, but it isn't a bug.

We discussed that earlier.

Duga

11:00 PM

[Vannevelj/VSDiagnostics] Hosch250 edited issue #503: Aggregate: Elementary methods

Jeroen Vannevel

We might have introduced a bug by making erroneous assumptions

Hosch250

Yeah.

I was referring to this comment:

Apr 22 at 20:54, by Hosch250

That is probably a bug.

Jeroen Vannevel

ah

I thought you meant it's probably causing a bug in our code

Hosch250

Are you interested in doing any analyzers for Roslyn?

I found a few issues that contain a bunch that I can point you to.

Jeroen Vannevel

what do you mean?

Hosch250

11:08 PM

Some of them are already done in VSD.

Jeroen Vannevel

for the actual Roslyn compiler?

Hosch250

Yeah.

Jeroen Vannevel

Not at the moment -- maybe one day

thanks though

Hosch250

OK. Apparently, you are supposed to talk with them before doing any new features.

Jeroen Vannevel

yeah, that's pretty common I'd say :P Always good to ask for pointers in a new codebase

Hosch250

11:11 PM

Got anything else you want me to do?

Jeroen Vannevel

everything is up to you. 182 issues to choose from if you see something you're particularly interested in

Duga

[Vannevelj/VSDiagnostics] Vannevelj closed issue #493: Avoid LINQ in analyzers

[Vannevelj/VSDiagnostics] Vannevelj merged pull request #532: Remove Linq from Analyzers from Hosch250/VSDiagnostics/Issue493 into Vannevelj/VSDiagnostics/develop

[Vannevelj/VSDiagnostics] Vannevelj pushed commit f5af7d73 to develop

Remove Linq from Analyzers (#532)

* Remove Linq from EnumCanHaveFlagsAttributeAnalyzer.

* Remove Linq from FlagsEnumValuesAreNotPowersOfTwoAnalyzer

* Remove Linq from OnPropertyChangedWithoutCallerMemberNameAnalyzer

* Implement FirstOrDefault.

* Remove Linq from EmptyArgumentExceptionAnalyzer

* Remove Linq from RethrowExceptionWithoutLosingStacktraceAnalyzer

* Remove Linq from EmptyCatchClauseAnalyzer

* Remove Linq from ArgumentExceptionWithoutNameofOperatorAnalyzer

*

Hosch250

I'd be interested in doing them all, but I'm contracted to do 450 hours on RD for my internship this summer.

I really learn a lot about C# doing VSD.

Jeroen Vannevel

yeah I've read it in the RD room -- it's an interesting internship

Duga

[Vannevelj/VSDiagnostics] build for commit f5af7d73 on develop: AppVeyor build succeeded

Transcript for