Give a diagnostic when creating a function in a class and not specifying the first argument as self

src/Analysis/Ast/Impl/Analyzer/Symbols/FunctionEvaluator.cs

@@ -55,12 +55,13 @@ public override void Evaluate() {
                 var returnType = TryDetermineReturnValue();
 
                 var parameters = Eval.CreateFunctionParameters(_self, _function, FunctionDefinition, !stub);
+                CheckValidOverload(parameters);
                 _overload.SetParameters(parameters);
 
                 // Do process body of constructors since they may be declaring
                 // variables that are later used to determine return type of other
                 // methods and properties.
-                var ctor = _function.Name.EqualsOrdinal("__init__") || _function.Name.EqualsOrdinal("__new__");
+                var ctor = _function.IsDunderInit() || _function.IsDunderNew();
                 if (ctor || returnType.IsUnknown() || Module.ModuleType == ModuleType.User) {
                     // Return type from the annotation is sufficient for libraries and stubs, no need to walk the body.
                     FunctionDefinition.Body?.Walk(this);
@@ -98,6 +99,110 @@ private IPythonType TryDetermineReturnValue() {
             return annotationType;
         }
 
+        private void CheckValidOverload(IReadOnlyList<IParameterInfo> parameters) {
+            if (_self?.MemberType == PythonMemberType.Class) {
+                switch (_function) {
+                    case IPythonFunctionType function:
+                        CheckValidFunction(function, parameters);
+                        break;
+                    case IPythonPropertyType property:
+                        CheckValidProperty(property, parameters);
+                        break;
+                }
+            }
+        }
+
+        private void CheckValidFunction(IPythonFunctionType function, IReadOnlyList<IParameterInfo> parameters) {
+            // Only give diagnostic errors on functions if the decorators are valid 
+            if (!function.HasValidDecorators(Eval)) {
+                return;
+            }
+
+            // Don't give diagnostics on functions defined in metaclasses
+            if (SelfIsMetaclass()) {
+                return;
+            }
+
+            // Static methods don't need any diagnostics
+            if (function.IsStatic) {
+                return;
+            }
+
+            // Otherwise, functions defined in classes must have at least one argument
+            if (parameters.IsNullOrEmpty()) {
+                var funcLoc = Eval.GetLocation(FunctionDefinition.NameExpression);
+                ReportFunctionParams(Resources.NoMethodArgument, ErrorCodes.NoMethodArgument, funcLoc);
+                return;
+            }
+
+            var param = parameters[0].Name;
+            var paramLoc = Eval.GetLocation(FunctionDefinition.Parameters[0]);
+            // If it is a class method check for cls
+            if (function.IsClassMethod && !param.Equals("cls")) {
+                ReportFunctionParams(Resources.NoClsArgument, ErrorCodes.NoClsArgument, paramLoc);
+            }
+
+            // If it is a method check for self
+            if (!function.IsClassMethod && !param.Equals("self")) {
+                ReportFunctionParams(Resources.NoSelfArgument, ErrorCodes.NoSelfArgument, paramLoc);
+            }
+        }
+
+        private void CheckValidProperty(IPythonPropertyType property, IReadOnlyList<IParameterInfo> parameters) {
+            // Only give diagnostic errors on properties if the decorators are valid 
+            if (!property.HasValidDecorators(Eval)) {
+                return;
+            }
+
+            // Don't give diagnostics on properties defined in metaclasses
+            if (SelfIsMetaclass()) {
+                return;
+            }
+
+            // No diagnostics on static and class properties 
+            if (property.IsStatic || property.IsClassMethod) {
+                return;
+            }
+
+            // Otherwise, properties defined in classes must have at least one argument
+            if (parameters.IsNullOrEmpty()) {
+                var propertyLoc = Eval.GetLocation(FunctionDefinition.NameExpression);
+                ReportFunctionParams(Resources.NoMethodArgument, ErrorCodes.NoMethodArgument, propertyLoc);
+                return;
+            }
+
+            var param = parameters[0].Name;
+            var paramLoc = Eval.GetLocation(FunctionDefinition.Parameters[0]);
+            // Only check for self on properties because static and class properties are invalid
+            if (!param.Equals("self")) {
+                ReportFunctionParams(Resources.NoSelfArgument, ErrorCodes.NoSelfArgument, paramLoc);
+            }
+        }
+
+        /// <summary>
+        /// Returns if the function is part of a metaclass definition 
+        /// e.g 
+        /// class A(type):
+        ///  def f(cls): ...
+        /// f is a metaclass function
+        /// </summary>
+        /// <returns></returns>
+        private bool SelfIsMetaclass() {
+            // Just allow all specialized types in Mro to avoid false positives
+            return _self.Mro.Any(b => b.IsSpecialized);
+        }
+
+        private void ReportFunctionParams(string message, string errorCode, LocationInfo location) {
+            Eval.ReportDiagnostics(
+                Eval.Module.Uri,
+                new DiagnosticsEntry(
+                    message.FormatInvariant(FunctionDefinition.Name),
+                    location.Span,
+                    errorCode,
+                    Parsing.Severity.Warning,
+                    DiagnosticSource.Analysis));
+        }
+
         public override bool Walk(AssignmentStatement node) {
             var value = Eval.GetValueFromExpression(node.Right) ?? Eval.UnknownType;
             foreach (var lhs in node.Left) {
@@ -119,10 +224,8 @@ public override bool Walk(ReturnStatement node) {
             var value = Eval.GetValueFromExpression(node.Expression);
             if (value != null) {
                 // although technically legal, __init__ in a constructor should not have a not-none return value
-                if (FunctionDefinition.Name.EqualsOrdinal("__init__") && _function.DeclaringType.MemberType == PythonMemberType.Class 
-                    && !value.IsOfType(BuiltinTypeId.NoneType)) { 
-
-                    Eval.ReportDiagnostics(Module.Uri, new Diagnostics.DiagnosticsEntry(
+                if (_function.IsDunderInit() && !value.IsOfType(BuiltinTypeId.NoneType)) {
+                    Eval.ReportDiagnostics(Module.Uri, new DiagnosticsEntry(
                             Resources.ReturnInInit,
                             node.GetLocation(Eval).Span,
                             ErrorCodes.ReturnInInit,

src/Analysis/Ast/Impl/Analyzer/Symbols/SymbolCollector.cs

@@ -138,6 +138,7 @@ private void AddOverload(FunctionDefinition fd, IPythonClassMember function, Act
                 // collection types cannot be determined as imports haven't been processed.
                 var overload = new PythonFunctionOverload(function, fd, _eval.GetLocationOfName(fd), fd.ReturnAnnotation?.ToCodeString(_eval.Ast));
                 addOverload(overload);
+
                 _table.Add(new FunctionEvaluator(_eval, overload));
             }
         }
@@ -165,19 +166,17 @@ private bool TryAddProperty(FunctionDefinition node, IPythonType declaringType)
             foreach (var d in decorators.OfType<NameExpression>()) {
                 switch (d.Name) {
                     case @"property":
-                        AddProperty(node, declaringType, false);
-                        return true;
                     case @"abstractproperty":
-                        AddProperty(node, declaringType, true);
+                        AddProperty(node, declaringType);
                         return true;
                 }
             }
             return false;
         }
 
-        private void AddProperty(FunctionDefinition fd, IPythonType declaringType, bool isAbstract) {
+        private void AddProperty(FunctionDefinition fd, IPythonType declaringType) {
             if (!(_eval.LookupNameInScopes(fd.Name, LookupOptions.Local) is PythonPropertyType existing)) {
-                existing = new PythonPropertyType(fd, _eval.GetLocationOfName(fd), declaringType, isAbstract);
+                existing = new PythonPropertyType(fd, _eval.GetLocationOfName(fd), declaringType);
                 // The variable is transient (non-user declared) hence it does not have location.
                 // Property type is tracking locations for references and renaming.
                 _eval.DeclareVariable(fd.Name, existing, VariableSource.Declaration);

src/Analysis/Ast/Impl/Diagnostics/ErrorCodes.cs

@@ -25,10 +25,14 @@ public static class ErrorCodes {
         public const string UndefinedVariable = "undefined-variable";
         public const string VariableNotDefinedGlobally= "variable-not-defined-globally";
         public const string VariableNotDefinedNonLocal = "variable-not-defined-nonlocal";
+        public const string NoSelfArgument = "no-self-argument";
+        public const string NoClsArgument = "no-cls-argument";
+        public const string NoMethodArgument = "no-method-argument";
         public const string ReturnInInit = "return-in-init";
         public const string TypingTypeVarArguments = "typing-typevar-arguments";
         public const string TypingNewTypeArguments = "typing-newtype-arguments";
         public const string TypingGenericArguments = "typing-generic-arguments";
         public const string InheritNonClass = "inherit-non-class";
+        public const string InvalidDecoratorCombination = "invalid-decorator-combination";
     }
 }

src/Analysis/Ast/Impl/Extensions/FunctionDefinitionExtensions.cs

@@ -0,0 +1,13 @@
+using Microsoft.Python.Analysis.Types;
+
+namespace Microsoft.Python.Analysis {
+    public static class ClassMemberExtensions {
+        public static bool IsDunderInit(this IPythonClassMember member) {
+            return member.Name == "__init__" && member.DeclaringType?.MemberType == PythonMemberType.Class;
+        }
+
+        public static bool IsDunderNew(this IPythonClassMember member) {
+            return member.Name == "__new__" && member.DeclaringType?.MemberType == PythonMemberType.Class;
+        }
+    }
+}

src/Analysis/Ast/Impl/Extensions/PythonFunctionExtensions.cs

@@ -14,16 +14,20 @@
 // permissions and limitations under the License.
 
 using System.Linq;
+using Microsoft.Python.Analysis.Analyzer;
+using Microsoft.Python.Analysis.Diagnostics;
 using Microsoft.Python.Analysis.Types;
 using Microsoft.Python.Analysis.Values;
 using Microsoft.Python.Core;
+using Microsoft.Python.Parsing;
+using Microsoft.Python.Parsing.Ast;
 
 namespace Microsoft.Python.Analysis.Extensions {
     public static class PythonFunctionExtensions {
-        public static bool IsUnbound(this IPythonFunctionType f) 
+        public static bool IsUnbound(this IPythonFunctionType f)
             => f.DeclaringType != null && f.MemberType == PythonMemberType.Function;
 
-        public static bool IsBound(this IPythonFunctionType f) 
+        public static bool IsBound(this IPythonFunctionType f)
             => f.DeclaringType != null && f.MemberType == PythonMemberType.Method;
 
         public static bool HasClassFirstArgument(this IPythonClassMember m)
@@ -34,5 +38,44 @@ public static IScope GetScope(this IPythonFunctionType f) {
             IScope gs = f.DeclaringModule.GlobalScope;
             return gs?.TraverseBreadthFirst(s => s.Children).FirstOrDefault(s => s.Node == f.FunctionDefinition);
         }
+
+        /// <summary>
+        /// Reports any decorator errors on function.
+        /// Returns true if the decorator combinations for the property are valid
+        /// </summary>
+        public static bool HasValidDecorators(this IPythonFunctionType f, IExpressionEvaluator eval) {
+            bool valid = true;
+            // If function is abstract, allow all decorators because it will be overridden
+            if (f.IsAbstract) {
+                return valid;
+            }
+
+            foreach (var dec in (f.FunctionDefinition?.Decorators?.Decorators).MaybeEnumerate().OfType<NameExpression>()) {
+                switch (dec.Name) {
+                    case @"staticmethod":
+                        if (f.IsClassMethod) {
+                            ReportInvalidDecorator(dec, Resources.InvalidDecoratorForFunction.FormatInvariant("Staticmethod", "class"), eval);
+                            valid = false;
+                        }
+                        break;
+                    case @"classmethod":
+                        if (f.IsStatic) {
+                            ReportInvalidDecorator(dec, Resources.InvalidDecoratorForFunction.FormatInvariant("Classmethod", "static"), eval);
+                            valid = false;
+                        }
+                        break;
+                }
+            }
+            return valid;
+        }
+
+        private static void ReportInvalidDecorator(NameExpression name, string errorMsg, IExpressionEvaluator eval) {
+            eval.ReportDiagnostics(eval.Module.Uri,
+                new DiagnosticsEntry(
+                    errorMsg, eval.GetLocation(name).Span,
+                    Diagnostics.ErrorCodes.InvalidDecoratorCombination,
+                    Severity.Warning, DiagnosticSource.Analysis
+            ));
+        }
     }
 }

src/Analysis/Ast/Impl/Extensions/PythonPropertyExtensions.cs

@@ -0,0 +1,60 @@
+// Copyright(c) Microsoft Corporation
+// All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the License); you may not use
+// this file except in compliance with the License. You may obtain a copy of the
+// License at http://www.apache.org/licenses/LICENSE-2.0
+//
+// THIS CODE IS PROVIDED ON AN  *AS IS* BASIS, WITHOUT WARRANTIES OR CONDITIONS
+// OF ANY KIND, EITHER EXPRESS OR IMPLIED, INCLUDING WITHOUT LIMITATION ANY
+// IMPLIED WARRANTIES OR CONDITIONS OF TITLE, FITNESS FOR A PARTICULAR PURPOSE,
+// MERCHANTABILITY OR NON-INFRINGEMENT.
+//
+// See the Apache Version 2.0 License for specific language governing
+// permissions and limitations under the License.
+
+using System.Linq;
+using Microsoft.Python.Analysis.Analyzer;
+using Microsoft.Python.Analysis.Diagnostics;
+using Microsoft.Python.Analysis.Types;
+using Microsoft.Python.Core;
+using Microsoft.Python.Parsing;
+using Microsoft.Python.Parsing.Ast;
+
+namespace Microsoft.Python.Analysis.Extensions {
+    public static class PythonPropertyExtensions {
+        /// <summary>
+        /// Returns true if the decorator combiantions for the property are valid
+        /// </summary>
+        public static bool HasValidDecorators(this IPythonPropertyType p, IExpressionEvaluator eval) {
+            bool valid = true;
+            // If property is abstract, allow all decorators because it will be overridden
+            if(p.IsAbstract) {
+                return valid;
+            }
+
+            foreach (var dec in (p.FunctionDefinition?.Decorators?.Decorators).MaybeEnumerate().OfType<NameExpression>()) {
+                switch (dec.Name) {
+                    case @"staticmethod":
+                        ReportInvalidDecorator(dec, Resources.InvalidDecoratorForProperty.FormatInvariant("Staticmethods"), eval);
+                        valid = false;
+                        break;
+                    case @"classmethod":
+                        ReportInvalidDecorator(dec, Resources.InvalidDecoratorForProperty.FormatInvariant("Classmethods"), eval);
+                        valid = false;
+                        break;
+                }
+            }
+            return valid;
+        }
+
+        private static void ReportInvalidDecorator(NameExpression name, string errorMsg, IExpressionEvaluator eval) {
+            eval.ReportDiagnostics(eval.Module.Uri,
+                new DiagnosticsEntry(
+                    errorMsg, eval.GetLocation(name).Span,
+                    Diagnostics.ErrorCodes.InvalidDecoratorCombination, 
+                    Severity.Warning, DiagnosticSource.Analysis
+            ));
+        }
+    }
+}

src/Analysis/Ast/Impl/Extensions/PythonTypeExtensions.cs

@@ -37,8 +37,5 @@ public static void TransferDocumentationAndLocation(this IPythonType s, IPythonT
                 dst.Location = src.Location;
             }
         }
-
-        public static bool IsConstructor(this IPythonClassMember m)
-            => m.Name.EqualsOrdinal("__init__") ||  m.Name.EqualsOrdinal("__new__");
     }
 }