Code cleanup in the Microsoft.CodeAnalysis project

[VladimirReshetnikov]

Code cleanup in the Microsoft.CodeAnalysis project:

• Use operator ?. where it simplifies code; replace conditional freeing or disposal using if to ?. syntax
• Use nameof(...) instead of string literals in exception; fixed several wrong parameter names and misused exceptions; fixed expected exceptions in tests
• Rewrote ternary ? : expressions in terms of simpler && expressions
• Remove unnecessary using directives
• Remove some dead code, unused private methods, unused private method parameters
• Remove unused local variables and redundant assignments (value is always overwritten, or assigned value is always same as before)
• Moved declarations of some locals to nested blocks where they are used; joined with assignments if possible
• Simplify several control flow constructs, reduce block nesting level
• Remove redundant casts (identity conversions)
• Remove unnecessary conditions; e.g. (uint)expr >= 0
• Replace local variables with consts in some places
• Replace usages of local variables with boolean literals if their value is statically known
• Remove unobserved increments of locals
• Simplifiy double negations
• Shorten LINQ method chains; e.g. seq.Where(...).FirstOrDefault() to seq.FirstOrDefault(...)
• Mark fields readonly where possible
• Mark methods static where possible
• Remove unnecessary unsafe modifiers
• Replace delegate creation expression with an implicit conversion from lambda
• Replace trivial lambdas with method groups
• Convert extension method invocation syntax from plain static to extension
• Replace array creation expressions with array initializers
• Replace is/cast combinations with as/null-check
• Add <see> and <paramref> to some XML doc comments; remove empty elements
• Remove case labels adjacent to default labels in switch statements; remove redundant default: break; sections
• Convert some ifs to ternary operators
• Add [StructLayout(LayoutKind.Auto)] for some structs
• Convert readonly field + property to a read-only property in some places
• Use => body in some properties
• Inline intermediate locals in some places

[VladimirReshetnikov]

This assignment is actually redundant: TryGetValue sets its out parameter to null if it returns false. I would like to remove the assignment and if completely. But I'm not sure if it would be unanimously accepted.

[cston]

It's reasonable to remove the assignment and if. There are many examples of that in the code.

[gafter]

I checked the specification of TryGetValue and that behavior is guaranteed.

[VladimirReshetnikov]

Ok. I'm going to remove the if.

[VladimirReshetnikov]

Done

[VladimirReshetnikov]

@gafter @jaredpar @VSadov @tmat @pharring @cston @amcasey @mavasani @agocke Please review.

[sharwell]

💡 It might seem counter-intuitive, but it would have been helpful if this (or future requests of a related nature) was submitted as separate pull requests for each specific type of change (yes that means ~2 dozen pull requests). When it comes to external reviewers that may or may not have time to look at the entire diff, sometimes people will want to focus their examination on particular topic of most interest to them, and it becomes very time consuming when everything is submitted together.

[sharwell]

❓ This is only a partial conversion to a documentation comment. Did you mean to convert the internal structure to its XML form as well?

[VladimirReshetnikov]

@sharwell Unfortunately, we have many doc comments that use indentation for formatting, that is only useful when you are looking at the source, but not preserved in the generated documentation or tooltips. We will consider fixing all of them as a separate effort later.

[VladimirReshetnikov]

Unobservable increment.

[gafter]

This pull request has become much too large to review, and keeps changing out from under my attempt to review it.

In the future can you please do separate refactorings seprately?

[VladimirReshetnikov]

@gafter I will try to keep PR sizes more manageable. I think this one is in its final form, unless there will be a strong feedback requesting to change something.

[tmat]

@VladimirReshetnikov Could we wait with this after RC is snapped?

[VladimirReshetnikov]

@tmat Sure. It means 3/17/2015 after noon?

[tannergooding]

@VladimirReshetnikov Should be. I'll be kicking off the signed build and can update you once I have done so.

[VladimirReshetnikov]

@tmat @tannergooding Is it safe to merge now?

[tmat]

Yes