Exclude attribute annotations from AnnotateDeclaration if not applicable

If there is no attributes context, the annotation does not make sanse.
This is primarily a concern regarding variables, since module variables can have attributes, but local variables cannot. Accordingly, the annotation target enum is not sufficiently granular to provide all information.

Author: MDoerner

Rubberduck.Core/UI/Converters/AnnotateDeclarationCommandCEVisibilityConverter.cs

@@ -32,6 +32,13 @@ private bool ShouldBeVisible(IAnnotation annotation, ICodeExplorerNode node)
                 return false;
             }
 
+            if (!target.DeclarationType.HasFlag(DeclarationType.Module)
+                && target.AttributesPassContext == null
+                && annotation is IAttributeAnnotation)
+            {
+                return false;
+            }
+
             var targetType = target.DeclarationType;
 
             switch (annotation.Target)
@@ -46,7 +53,7 @@ private bool ShouldBeVisible(IAnnotation annotation, ICodeExplorerNode node)
                     return targetType.HasFlag(DeclarationType.Variable)
                            || targetType.HasFlag(DeclarationType.Constant);
                 case AnnotationTarget.General:
-                    return true;
+                    return !targetType.HasFlag(DeclarationType.Module);
                 case AnnotationTarget.Identifier:
                     return false;
                 default:

Rubberduck.Core/UI/Refactorings/AnnotateDeclaration/AnnotateDeclarationViewModel.cs

@@ -35,8 +35,11 @@ IAnnotationArgumentViewModelFactory argumentFactory
         private static IReadOnlyList<IAnnotation> AnnotationsForDeclaration(Declaration declaration, IEnumerable<IAnnotation> annotations)
         {
             return AnnotationsForDeclarationType(declaration.DeclarationType, annotations)
-                .Where(annotation => annotation.AllowMultiple 
-                                     || !declaration.Annotations.Any(pta => annotation.Equals(pta.Annotation)))
+                .Where(annotation => (annotation.AllowMultiple 
+                                        || !declaration.Annotations.Any(pta => annotation.Equals(pta.Annotation)))
+                                     && (declaration.DeclarationType.HasFlag(DeclarationType.Module) 
+                                        || declaration.AttributesPassContext != null 
+                                        || !(annotation is IAttributeAnnotation)))
                 .ToList();
         }
 

RubberduckTests/Refactoring/AnnotateDeclaration/AnnotateDeclarationViewModelTests.cs

@@ -53,7 +53,40 @@ public void AnnotationAlreadyPresent_AllowsMultiple_InApplicableAnnotations()
 
             Assert.True(applicableAnnotationNames.Contains("Ignore"));
         }
-        
+
+        [Test]
+        public void AttributeAnnotation_NoAttributesContext_NoModule_NotInApplicableAnnotations()
+        {
+            var viewModel = TestViewModel(DeclarationType.Variable, localScope: true);
+            var applicableAnnotationNames = viewModel.ApplicableAnnotations
+                .Select(annotation => annotation.Name)
+                .ToList();
+
+            Assert.False(applicableAnnotationNames.Contains("VariableDescription"));
+        }
+
+        [Test]
+        public void AttributeAnnotation_NoAttributesContext_IsModule_InApplicableAnnotations()
+        {
+            var viewModel = TestViewModel(DeclarationType.Module);
+            var applicableAnnotationNames = viewModel.ApplicableAnnotations
+                .Select(annotation => annotation.Name)
+                .ToList();
+
+            Assert.True(applicableAnnotationNames.Contains("Exposed"));
+        }
+
+        [Test]
+        public void AttributeAnnotation_HasAttributesContext_NotInApplicableAnnotations()
+        {
+            var viewModel = TestViewModel(DeclarationType.Variable, localScope: false);
+            var applicableAnnotationNames = viewModel.ApplicableAnnotations
+                .Select(annotation => annotation.Name)
+                .ToList();
+
+            Assert.True(applicableAnnotationNames.Contains("VariableDescription"));
+        }
+
         [Test]
         public void AnnotationNull_Invalid()
         {
@@ -264,10 +297,10 @@ public void DialogOK_SetsArguments()
         }
 
 
-        private AnnotateDeclarationViewModel TestViewModel(DeclarationType targetDeclarationType, IAnnotation initialAnnotation = null)
+        private AnnotateDeclarationViewModel TestViewModel(DeclarationType targetDeclarationType, IAnnotation initialAnnotation = null, bool localScope = false)
         {
             var argumentFactory = MockArgumentFactory().Object;
-            return TestViewModel(targetDeclarationType, argumentFactory, initialAnnotation);
+            return TestViewModel(targetDeclarationType, argumentFactory, initialAnnotation, localScope);
         }
 
         private Mock<IAnnotationArgumentViewModelFactory> MockArgumentFactory(IReadOnlyList<bool> hasErrorSpecifications = null)
@@ -296,20 +329,21 @@ private Mock<IAnnotationArgumentViewModel> MockArgument(AnnotationArgumentType a
             return mockArgument;
         }
 
-        private AnnotateDeclarationViewModel TestViewModel(DeclarationType targetDeclarationType, IAnnotationArgumentViewModelFactory argumentFactory, IAnnotation initialAnnotation = null)
+        private AnnotateDeclarationViewModel TestViewModel(DeclarationType targetDeclarationType, IAnnotationArgumentViewModelFactory argumentFactory, IAnnotation initialAnnotation = null, bool localScope = false)
         {
-            var targetDeclaration = TestDeclaration(targetDeclarationType);
+            var targetDeclaration = TestDeclaration(targetDeclarationType, localScope);
             var model = new AnnotateDeclarationModel(targetDeclaration, initialAnnotation);
             return new AnnotateDeclarationViewModel(model, _testAnnotations, argumentFactory);
         }
 
-        private Declaration TestDeclaration(DeclarationType targetDeclarationType)
+        private Declaration TestDeclaration(DeclarationType targetDeclarationType, bool localScope = false)
         {
             const string code = @"
 Public myVar As Variant
 
 '@Ignore MissingMemberAnnotationInspection
 Public Sub Foo
+Dim bar As Variant
 End Sub
 
 '@DefaultMember
@@ -319,7 +353,15 @@ End Function
             var vbe = MockVbeBuilder.BuildFromSingleModule(code, ComponentType.ClassModule, out _).Object;
             using (var state = MockParser.CreateAndParse(vbe))
             {
-                return state.DeclarationFinder.UserDeclarations(targetDeclarationType).Single();
+                if (localScope)
+                {
+                    return state.DeclarationFinder.UserDeclarations(targetDeclarationType)
+                        .Single(declaration => declaration.ParentScopeDeclaration.DeclarationType.HasFlag(DeclarationType.Member));
+                }
+
+                return state.DeclarationFinder.UserDeclarations(targetDeclarationType)
+                    .Single(declaration => declaration.DeclarationType.HasFlag(DeclarationType.Module) 
+                                           || declaration.ParentScopeDeclaration.DeclarationType.HasFlag(DeclarationType.Module));
             }
         }