New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EnvDTE.CodeElement.GetStartPoint(part)/GetEndPoint(part) not implemented for many parts #2461

Open
carlos-quintero opened this Issue May 2, 2015 · 3 comments

Comments

Projects
None yet
3 participants
@carlos-quintero

carlos-quintero commented May 2, 2015

When you use the EnvDTE.ProjectItem.FileCodeModel to call EnvDTE.CodeElement.GetStartPoint(part) and EnvDTE.CodeElement.GetEndPoint(part), many calls fail as "not implemented"

Steps to reproduce the problem:

  1. Create a package with a menu command that uses this code:
  private void MenuItemCallback(object sender, EventArgs e)
  {
     try
     {
        EnvDTE.DTE dte = this.ServiceProvider.GetService(typeof(EnvDTE.DTE)) as EnvDTE.DTE;
        ShowCodeElements(dte.ActiveDocument.ProjectItem.FileCodeModel.CodeElements);
     }
     catch (Exception ex)
     {
        System.Windows.Forms.MessageBox.Show(ex.ToString());
     }
  }

  private void ShowCodeElements(EnvDTE.CodeElements codeElements)
  {
     foreach (EnvDTE.CodeElement codeElement in codeElements)
     {
        ShowCodeElement(codeElement);
     }
  }

  private void ShowCodeElement(EnvDTE.CodeElement codeElement)
  {
     ShowStartEndTextPoints(codeElement);

     switch (codeElement.Kind)
     {
        case EnvDTE.vsCMElement.vsCMElementNamespace:

           EnvDTE.CodeNamespace codeNamespace = codeElement as EnvDTE.CodeNamespace;

           ShowCodeElements(codeNamespace.Members);
           break;

        case EnvDTE.vsCMElement.vsCMElementProperty:

           EnvDTE.CodeProperty codeProperty = codeElement as EnvDTE.CodeProperty;

           try
           {
              ShowCodeElement(codeProperty.Getter as EnvDTE.CodeElement);
           }
           catch
           {
           }
           try
           {
              ShowCodeElement(codeProperty.Setter as EnvDTE.CodeElement);
           }
           catch
           {
           }
           break;

        case EnvDTE.vsCMElement.vsCMElementEvent:

           EnvDTE80.CodeEvent codeEvent = codeElement as EnvDTE80.CodeEvent;
           try
           {
              ShowStartEndTextPoints(codeEvent.Adder as EnvDTE.CodeElement);
           }
           catch
           {
           }
           try
           {
              ShowStartEndTextPoints(codeEvent.Remover as EnvDTE.CodeElement);
           }
           catch
           {
           }
           try
           {
              ShowStartEndTextPoints(codeEvent.Thrower as EnvDTE.CodeElement);
           }
           catch
           {
           }
           break;

        case EnvDTE.vsCMElement.vsCMElementFunction:

           EnvDTE.CodeFunction codeFunction = codeElement as EnvDTE.CodeFunction;
           ShowCodeElements(codeFunction.Parameters);
           break;

        default:

           EnvDTE.CodeType codeType = codeElement as EnvDTE.CodeType;
           if (codeType != null)
           {
              ShowCodeElements(codeType.Members);
           }
           break;
     }
  }

  private void ShowStartEndTextPoints(EnvDTE.CodeElement codeElement)
  {
     if (codeElement != null)
     {
        ShowStartEndTextPoints(codeElement, EnvDTE.vsCMPart.vsCMPartWholeWithAttributes);
        ShowStartEndTextPoints(codeElement, EnvDTE.vsCMPart.vsCMPartWhole);
        ShowStartEndTextPoints(codeElement, EnvDTE.vsCMPart.vsCMPartHeaderWithAttributes);
        ShowStartEndTextPoints(codeElement, EnvDTE.vsCMPart.vsCMPartHeader);
        ShowStartEndTextPoints(codeElement, EnvDTE.vsCMPart.vsCMPartName);
     }
  }

  private void ShowStartEndTextPoints(EnvDTE.CodeElement codeElement, EnvDTE.vsCMPart part)
  {
     EnvDTE.TextPoint textPoint;
     string info;

     info = "Kind: " + codeElement.Kind.ToString() + "\nName: " + codeElement.Name + "\nPart: " + part.ToString() + "\nPoint: ";

     try
     {
        textPoint = codeElement.GetStartPoint(part);
        System.Windows.Forms.MessageBox.Show("Start\n" + info + textPoint.Line.ToString() + "," + textPoint.LineCharOffset.ToString(),
           "", System.Windows.Forms.MessageBoxButtons.OK, System.Windows.Forms.MessageBoxIcon.Information);
     }
     catch (Exception ex)
     {
        System.Windows.Forms.MessageBox.Show("Start\n" + info + " " + ex.Message, "", System.Windows.Forms.MessageBoxButtons.OK, 
           System.Windows.Forms.MessageBoxIcon.Error);
     }

     try
     {
        textPoint = codeElement.GetEndPoint(part);
        System.Windows.Forms.MessageBox.Show("End\n" + info + textPoint.Line.ToString() + "," + textPoint.LineCharOffset.ToString(),
           "", System.Windows.Forms.MessageBoxButtons.OK, System.Windows.Forms.MessageBoxIcon.Information);
     }
     catch (Exception ex)
     {
        System.Windows.Forms.MessageBox.Show("End\n" + info + " " + ex.Message, "", System.Windows.Forms.MessageBoxButtons.OK, 
           System.Windows.Forms.MessageBoxIcon.Error);
     }
  }
  1. Run the package and in the second VS instance, create a C# Class Library with this code:

namespace Namespace1
{

public class SkipReview : System.Attribute
{
}

[SkipReview]
public delegate void Delegate1(int p1);

[SkipReview]
public interface Interface1
{

  [SkipReview]
  void Function1();

  [SkipReview]
  event Delegate1 Event1;

  [SkipReview]
  int Property1 { get; set; }

}

[SkipReview]
public class Class1 : Interface1
{

  [SkipReview]
  const int CONST1 = 0;

  [SkipReview]
  int Field1 = 0;

  [SkipReview]
  void Interface1.Function1() 
  {
  }

  [SkipReview]
  event Delegate1 Interface1.Event1
  {
     [SkipReview]
     add
     {
     }
     [SkipReview]
     remove
     {
     }
  }

  [SkipReview]
  int Interface1.Property1
  {
     [SkipReview]
     get
     {
        return Field1;
     }
     [SkipReview]
     set
     {
     }
  }

  [SkipReview]
  void Function2()
  {
  }

  [SkipReview]
  event Delegate1 Event2
  {
     [SkipReview]
     add
     {
     }
     [SkipReview]
     remove
     {
     }
  }

  [SkipReview]
  int Property2
  {
     [SkipReview]
     get
     {
        return 0;
     }
     [SkipReview]
     set
     {
     }
  }

}

[SkipReview]
public struct Struct1
{

  [SkipReview]
  const int CONST1 = 0;

  [SkipReview]
  int Field1;

  [SkipReview]
  void Function2()
  {
  }

  [SkipReview]
  event System.EventHandler Event2
  {
     [SkipReview]
     add
     {
     }
     [SkipReview]
     remove
     {
     }
  }

  [SkipReview]
  int Property2
  {
     [SkipReview]
     get
     {
        Field1 = 0;
        return Field1;
     }

     [SkipReview]
     set
     {
     }

  }

}

[SkipReview]
public enum Enum1
{

  [SkipReview]
  EnumItem1 = 1,

  [SkipReview]
  EnumItem2 = 2

}

}

  1. Click the package command. You will find lots of "not implemented" exceptions. Notice that:

vsCMPart.vsCMPartWholeWithAttributes
vsCMPart.vsCMPartWhole
vsCMPart.vsCMPartHeaderWithAttributes
vsCMPart.vsCMPartHeader
vsCMPartName

should be "valid" parts (ie, "implemented") for any kind of code element.

vsCMPartBody (not shown in this report) may fail for code elements that lack body (parameters, members of interfaces, abstract method/properties, etc.)

Test also with VB.NET code

@Pilchie

This comment has been minimized.

Show comment
Hide comment
@Pilchie

Pilchie May 6, 2015

Member

Hey @carlos-j-quintero - can you let us know whether these are really compatibility issues? From @basoundr's research, it looks like we threw the same exceptions in VS2013. While I know it would be nice to fix them, I am concerned about making changes like that so close to the end of VS2015, without giving time for other extensions to test with the changes.

Member

Pilchie commented May 6, 2015

Hey @carlos-j-quintero - can you let us know whether these are really compatibility issues? From @basoundr's research, it looks like we threw the same exceptions in VS2013. While I know it would be nice to fix them, I am concerned about making changes like that so close to the end of VS2015, without giving time for other extensions to test with the changes.

@carlos-quintero

This comment has been minimized.

Show comment
Hide comment
@carlos-quintero

carlos-quintero May 7, 2015

Hi @Pilchie @basoundr ,

On the one hand, these are not really compatibility issues because the GetStartPoint/GetEndPoint methods have been buggy since VS.NET 2002... Any add-in or package using them has to apply a lot of workarounds and patches on top of them to get the correct results, either because they are not implemented, or they cause COMException in some cases, or they return an incorrect result. So, it would be very nice that at some point they are fully implemented and reliable.

On the other hand, my extension has all the needed workarounds and patches implemented and as of VS 2015 RC all my integration tests pass. I have had to implement new workarounds specific to VS 2015 for bugs that I have reported but I will be able to remove them once your last fixes are available in VS 2015 RTM (such as #2437, #2355). Notice that most workarounds about GetStartPoint/GetEndPoint rely on CodeElement.Name returning the correct result even on edge cases (such as #2437, which is now fixed but I was able to workaround in VS 2015 RC).

Notice that GetStartPoint/GetEndPoint needs unit tests for 6 vsCMPart (whole with attributes, whole, header with attributes, header, name and body), 2 locations (start/end), 2 languages (VB.NET and C#), many code element kinds (namespace...enum item) and lots of edge cases.

Bottom line: I would prefer you to fix NEITHER this bug NOR #857 (which is a compatibility break still pending but I have a workaround) until after VS 2015 RTM, for the risk of breaking what now works (although with workarounds).

carlos-quintero commented May 7, 2015

Hi @Pilchie @basoundr ,

On the one hand, these are not really compatibility issues because the GetStartPoint/GetEndPoint methods have been buggy since VS.NET 2002... Any add-in or package using them has to apply a lot of workarounds and patches on top of them to get the correct results, either because they are not implemented, or they cause COMException in some cases, or they return an incorrect result. So, it would be very nice that at some point they are fully implemented and reliable.

On the other hand, my extension has all the needed workarounds and patches implemented and as of VS 2015 RC all my integration tests pass. I have had to implement new workarounds specific to VS 2015 for bugs that I have reported but I will be able to remove them once your last fixes are available in VS 2015 RTM (such as #2437, #2355). Notice that most workarounds about GetStartPoint/GetEndPoint rely on CodeElement.Name returning the correct result even on edge cases (such as #2437, which is now fixed but I was able to workaround in VS 2015 RC).

Notice that GetStartPoint/GetEndPoint needs unit tests for 6 vsCMPart (whole with attributes, whole, header with attributes, header, name and body), 2 locations (start/end), 2 languages (VB.NET and C#), many code element kinds (namespace...enum item) and lots of edge cases.

Bottom line: I would prefer you to fix NEITHER this bug NOR #857 (which is a compatibility break still pending but I have a workaround) until after VS 2015 RTM, for the risk of breaking what now works (although with workarounds).

@Pilchie Pilchie modified the milestones: Unknown, 1.0 (stable) May 7, 2015

@Pilchie

This comment has been minimized.

Show comment
Hide comment
@Pilchie

Pilchie May 7, 2015

Member

Moving this bug out of 1.0, because of the risk of changing this so late in the cycle.

Member

Pilchie commented May 7, 2015

Moving this bug out of 1.0, because of the risk of changing this so late in the cycle.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment