views:

986

answers:

3

I tried to flag a collection property on a class as Obsolete to find all the occurances and keep a shrinking list of things to fix in my warning-list, due to the fact that we need to replace this collection property with something else.


Edit: I've submitted this through Microsoft Connect, issue #417159.

I will post an update if I get a response.


However, to my surprise, the list only contained a few occurances, far fewer than I knew there were, and spotchecks tells me that for some reason, the usage of the property isn't always flagged as obsolete by the compiler in the warning list.

Here's an example program, ready to compile in Visual Studio 2008.

Note the four lines near the end tagged with #1-#4, of these, I'd expect all of them to report that the property used was obsolete, but #3 isn't, and it seems that if I just move on to the collection properties or methods directly, the usage of the property itself isn't flagged as obsolete. Note that #3 and #4 is referencing the same property, and #4 is flagged as using an obsolete property, whereas #3 isn't. Tests shows that if, in the expression, I access properties or methods of the collection the property returns, the compiler doesn't complain.

Is this a bug, or is this a "hidden gem" of the C# compiler I wasn't aware of?

using System;
using System.Collections.Generic;

namespace TestApp
{
    public abstract class BaseClass
    {
        [Obsolete]
        public abstract String Value
        {
            get;
        }

        [Obsolete]
        public abstract String[] ValueArray
        {
            get;
        }

        [Obsolete]
        public abstract List<String> ValueList
        {
            get;
        }
    }

    public class DerivedClass : BaseClass
    {
        [Obsolete]
        public override String Value
        {
            get
            {
                return "Test";
            }
        }

        [Obsolete]
        public override String[] ValueArray
        {
            get
            {
                return new[] { "A", "B" };
            }
        }

        [Obsolete]
        public override List<String> ValueList
        {
            get
            {
                return new List<String>(new[] { "A", "B" });
            }
        }
    }

    public class Program
    {
        public static void Main(String[] args)
        {
            BaseClass bc = new DerivedClass();
            Console.Out.WriteLine(bc.Value);             // #1 - OK
            Console.Out.WriteLine(bc.ValueArray.Length); // #2 - OK
            Console.Out.WriteLine(bc.ValueList.Count);   // #3 - Not OK?
            List<String> list = bc.ValueList;            // #4 - OK
        }
    }
}
+10  A: 

Hmm... looks like a compiler bug to me! It fails the following (ECMA 334v4):

24.4.3 The Obsolete attribute The attribute Obsolete is used to mark types and members of types that should no longer be used. If a program uses a type or member that is decorated with the Obsolete attribute, then the compiler shall issue a warning or error in order to alert the developer, so the offending code can be fixed. Specifically, the compiler shall issue a warning if no error parameter is provided, or if the error parameter is provided and has the value false. The compiler shall issue a compile-time error if the error parameter is specified and has the value true.

In particular, when marked true it should issue an error, and it doesn't. Good find! You could report it on "connect", or if you don't want the pain of setting up a login, let me know and I'll happily log it (referencing your post here; no attempt to "steal" anything).

(update)

Reduced code to reproduce:

using System;
using System.Collections.Generic;
static class Program {
    static void Main() {
        int count = Test.Count;
    }

    [Obsolete("Should error", true)]
    public static List<string> Test {
        get {throw new NotImplementedException();}
    }
}

Note that mono 2.0 gets it right, as does the MS C# 2.0 compiler. It is only the MS C# 3.0 (.NET 3.5) compiler that is broken.

Marc Gravell
Ok, good, that confirms my suspicions, I've edited my question and posted the link to the connect issue.
Lasse V. Karlsen
+2  A: 

I agree with Marc: it looks like a compiler bug. Interestingly, gmcs (the Mono C# compiler) gets it right:

Test.cs(65,26): warning CS0219: The variable `list' is assigned but its value is never used
Test.cs(62,38): warning CS0612: `TestApp.BaseClass.Value' is obsolete
Test.cs(63,38): warning CS0612: `TestApp.BaseClass.ValueArray' is obsolete
Test.cs(64,38): warning CS0612: `TestApp.BaseClass.ValueList' is obsolete
Test.cs(65,36): warning CS0612: `TestApp.BaseClass.ValueList' is obsolete
Compilation succeeded - 5 warning(s)
Jon Skeet
As does MS 2.0; but funny how we both tried "gmcs" first...
Marc Gravell
Can only accept one, so I'm accepting Marc's as he was first :)
Lasse V. Karlsen
+11  A: 

This is a genuine bug. Unfortunately due to a refactoring clean up that missed this case. I fixed this for the C# 4.0 compiler release coming up in VS 2010/NDP 4.0 but there are no plans to fix it now in Orcas and unfortunately there is no work around I know of to deal with this.

I hate to say it but you will need to upgrade to the NDP 4 csc.exe or VS2010 when they become available to fix this issue.

I'm thinking about posting an entry on my fresh new msdn blog about this. Makes a good anecdotal example of how refactoring can break your code.

Ian Halliday

C# Compiler SDE
Microsoft

Ian Halliday
Thank you for taking the time to post that here. Appreciated.
Marc Gravell
Yes indeed. Another milestone for Stack Overflow, I think :)
Jon Skeet