bkoelman / ResharperCodeContractNullability

Reports diagnostics, helping you to annotate your source tree with (Item)NotNull / (Item)CanBeNull attributes.
https://www.jetbrains.com/resharper/help/Code_Analysis__Code_Annotations.html
Apache License 2.0
26 stars 4 forks source link

Namspace import after file header #19

Closed sushiat closed 8 years ago

sushiat commented 8 years ago

Hi, I've got one small improvement idea: when importing the "JetBrains.Annotations" namespace when performing a code fix would be possible to scan the first lines of the file for file header comments and place it afterwards?

I know it sounds like a minor issue but ReSharper screws up when running code cleanup (seems to run things in the wrong order, seems to check file header before re-ordering of namespaces):

After code fix:

using JetBrains.Annotations;
// --------------------------------------------------------------------------------------------------------------------
// <copyright file="FileName.cs" company="Letterkenny Institute of Technology">
// Copyright (C) Letterkenny Institute of Technology 2007-2016
// </copyright>
// --------------------------------------------------------------------------------------------------------------------

namespace NameSpace
{
    using AnyOtherNamespaces;

After Resharper code cleanup:

// --------------------------------------------------------------------------------------------------------------------
// <copyright file="FileName.cs" company="Letterkenny Institute of Technology">
// Copyright (C) Letterkenny Institute of Technology 2007-2016
// </copyright>
// --------------------------------------------------------------------------------------------------------------------

 // --------------------------------------------------------------------------------------------------------------------
// <copyright file="FileName.cs" company="Letterkenny Institute of Technology">
// Copyright (C) Letterkenny Institute of Technology 2007-2016
// </copyright>
// --------------------------------------------------------------------------------------------------------------------

namespace NameSpace
{
    using JetBrains.Annotations;
    using AnyOtherNamespaces;

I'm currently "migrating" a ton of code using your excellent analyzers but it means I have to manually move that line down in every single file before cleaning up....

bkoelman commented 8 years ago

Hi Markus, thanks for reporting this. The coming weeks I won't have access to a computer as I'm on vacation. So fixing this may take some time. However there may be a faster route.

This issue is caused by: https://github.com/bkoelman/ResharperCodeContractNullability/blob/master/src/CodeContractNullability/CodeContractNullability/BaseCodeFixProvider.cs#l140

It calls into the Roslyn framework to add the namespace import. Perhaps you can file a bug at https://github.com/dotnet/roslyn/issues or try to fix the code at https://github.com/dotnet/roslyn/blob/a4e375b95953e471660e9686a46893c97db70b0e/src/Workspaces/Core/Portable/Editing/ImportAdder.cs .

Hope that helps.

sushiat commented 8 years ago

Will investigate today and update the issue here once I checked it out.

sushiat commented 8 years ago

Sorry for only updating this now, but hopefully you are enjoying your vacation anyway. I did look at the Roslyn code but man is that project a beast. I navigated around and finally found the actual line where the imports are generated and inserted but without the slightest idea on how to change it so it would be doing it after the file header :)

I did therefore raise an issue over at the Roslyn project: https://github.com/dotnet/roslyn/issues/8797

bkoelman commented 8 years ago

I am at Miami & The Keys, which is great! Feel free to add updates, I'll see what I can do whenever I have wifi.

Yes the Roslyn codebase can be hard to read at times. Some basic concepts are explained at https://github.com/dotnet/roslyn/wiki/Roslyn%20Overview. In this case, I would suggest something like:

  1. Find existing syntax node to insert Using Statement above.
  2. Cut off it's leading trivia (= white space and comments)
  3. Insert new syntax for using statement
  4. Add the cut leading trivia to the new node.

Note that all these are immutable data structures, so operations return a new instance with the changes.

Alternatively, you may want to comment out the offending line, rebuild the analyzer and run it during your migration. That breaks your build, but is easily fixed afterwards using Resharper's solution-wide "add missing usings" fix.

bkoelman commented 8 years ago

I have submitted a PR that fixes this issue.

bkoelman commented 8 years ago

@markuskorbel The related fix in roslyn has been merged. This should light up in the next VS release.