Lint on no method argument, no self argument, no cls argument (#1557)

Lint on no method argument, no self argument, no cls argument (#1557)

README.md

@@ -27,13 +27,16 @@ The following diagnostics are supported:
 | `inherit-non-class` | Attempted to inherit something that is not a class. |
 | `too-many-function-arguments` | Too many arguments have been provided to a function call. |
 | `too-many-positional-arguments-before-star` | Too many arguments have been provided before a starred argument. |
+| `no-cls-argument` | First parameter in a class method must be `cls` |
+| `no-method-argument` | Method has no arguments
+| `no-self-argument` | First parameter in a method must be `self`
 | `parameter-already-specified` | A argument with this name has already been specified. |
 | `parameter-missing` | A required positional argument is missing. |
 | `positional-argument-after-keyword` | A positional argument has been provided after a keyword argument. |
 | `return-in-init` | Encountered an explicit return in `__init__` function. |
 | `typing-generic-arguments` | An error occurred while constructing `Generic`. |
-| `typing-typevar-arguments` | An error occurred while constructing `TypeVar`. |
 | `typing-newtype-arguments` | An error occurred while constructing `NewType`. |
+| `typing-typevar-arguments` | An error occurred while constructing `TypeVar`. |
 | `unknown-parameter-name` | The keyword argument name provided is unknown. |
 | `unresolved-import` | An import cannot be resolved, and may be missing. |
 | `undefined-variable` | A variable has been used that has not yet been defined. |

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

@@ -55,6 +55,7 @@ public FunctionEvaluator(ExpressionEval eval, PythonFunctionOverload overload)
                 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
@@ -105,6 +106,59 @@ public FunctionEvaluator(ExpressionEval eval, PythonFunctionOverload overload)
             return annotationType;
         }
 
+        private void CheckValidOverload(IReadOnlyList<IParameterInfo> parameters) {
+            if (_self?.MemberType == PythonMemberType.Class) {
+                switch (_function) {
+                    case IPythonFunctionType function:
+                        CheckValidFunction(function, parameters);
+                        break;
+                    //TODO check properties
+                }
+            }
+        }
+
+        private void CheckValidFunction(IPythonFunctionType function, IReadOnlyList<IParameterInfo> parameters) {
+            // Don't give diagnostics on functions defined in metaclasses
+            if (_self.IsMetaclass()) {
+                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 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) {

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

@@ -30,5 +30,8 @@ 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";
     }
 }

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

@@ -42,7 +42,7 @@ public static string GetTypeName(this BuiltinTypeId id, Version version)
         public static string GetTypeName(this BuiltinTypeId id, PythonLanguageVersion languageVersion)
             => id.GetTypeName(languageVersion.IsNone() || languageVersion.Is3x());
 
-        private static string GetTypeName(this BuiltinTypeId id, bool is3x) {
+        public static string GetTypeName(this BuiltinTypeId id, bool is3x) {
             string name;
             switch (id) {
                 case BuiltinTypeId.Bool: name = "bool"; break;

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

@@ -13,6 +13,7 @@
 // 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.Specializations.Typing;
 using Microsoft.Python.Analysis.Types;
@@ -58,6 +59,13 @@ public static class PythonClassExtensions {
             return unmangledName.StartsWithOrdinal("__") && memberName.EqualsOrdinal($"_{cls.Name}{unmangledName}");
         }
 
+        /// <summary>
+        /// Returns if the class is a metaclass. Metaclasses have 'type' in their Mro.
+        /// </summary>
+        public static bool IsMetaclass(this IPythonClassType cls) {
+            return cls.Mro.Any(b => b.Name == BuiltinTypeId.Type.GetTypeName(true));
+        }
+
         /// <summary>
         /// Gets specific type for the given generic type parameter, resolving bounds as well
         /// </summary>

src/Analysis/Ast/Impl/Resources.resx

@@ -195,6 +195,9 @@
   <data name="TypeVarSingleConstraint" xml:space="preserve">
     <value>A single constraint to TypeVar is not allowed.</value>
   </data>
+  <data name="NoSelfArgument" xml:space="preserve">
+    <value>Method '{0}' should have 'self' as first argument.</value>
+  </data>
   <data name="NewTypeFirstArgument" xml:space="preserve">
     <value>The first argument to NewType must be a string.</value>
   </data>
@@ -210,4 +213,10 @@
   <data name="InheritNonClass" xml:space="preserve">
     <value>Inheriting '{0}', which is not a class.</value>
   </data>
-</root>
+  <data name="NoClsArgument" xml:space="preserve">
+    <value>Class method '{0}' should have 'cls' as first argument.</value>
+  </data>
+  <data name="NoMethodArgument" xml:space="preserve">
+    <value>Method '{0}' has no argument.</value>
+  </data>
+</root>

src/Analysis/Ast/Impl/Types/PythonFunctionType.cs

@@ -26,6 +26,7 @@
 namespace Microsoft.Python.Analysis.Types {
     [DebuggerDisplay("Function {Name} ({TypeId})")]
     internal sealed class PythonFunctionType : PythonType, IPythonFunctionType {
+        private static readonly IReadOnlyList<string> DefaultClassMethods = new[] { "__new__", "__init_subclass__", "__class_getitem__" };
         private ImmutableArray<IPythonFunctionOverload> _overloads = ImmutableArray<IPythonFunctionOverload>.Empty;
         private bool _isAbstract;
         private bool _isSpecialized;
@@ -71,6 +72,7 @@ Location location
 
             location.Module.AddAstNode(this, fd);
             ProcessDecorators(fd);
+            DecideClassMethod();
         }
 
         #region IPythonType
@@ -127,9 +129,17 @@ internal void AddOverload(IPythonFunctionOverload overload)
             return new PythonUnboundMethod(this);
         }
 
+        private void DecideClassMethod() {
+            if (IsClassMethod) {
+                return;
+            }
+
+            IsClassMethod = DefaultClassMethods.Contains(Name);
+        }
+
         private void ProcessDecorators(FunctionDefinition fd) {
+            // TODO warn about incompatible combinations, e.g @staticmethod + @classmethod
             foreach (var dec in (fd.Decorators?.Decorators).MaybeEnumerate().OfType<NameExpression>()) {
-                // TODO: warn about incompatible combinations.
                 switch (dec.Name) {
                     case @"staticmethod":
                         IsStatic = true;

src/Analysis/Ast/Test/LintInheritNonClassTests.cs

@@ -202,7 +202,7 @@ class B:
 from module1 import B
 
 class C(B):
-    def hello():
+    def hello(self):
         pass
 ";
 

src/Analysis/Ast/Test/LintNewTypeTests.cs

@@ -100,7 +100,7 @@ public void TestInitialize()
 from typing import NewType
 
 class X:
-    def hello():
+    def hello(self):
         pass
 
 h = X()