diff --git a/src/Analysis/Ast/Impl/Analyzer/Symbols/SymbolCollector.cs b/src/Analysis/Ast/Impl/Analyzer/Symbols/SymbolCollector.cs index fe45a8fb9..2a70d6b56 100644 --- a/src/Analysis/Ast/Impl/Analyzer/Symbols/SymbolCollector.cs +++ b/src/Analysis/Ast/Impl/Analyzer/Symbols/SymbolCollector.cs @@ -17,6 +17,7 @@ using System.Collections.Generic; using System.Linq; using Microsoft.Python.Analysis.Analyzer.Evaluation; +using Microsoft.Python.Analysis.Diagnostics; using Microsoft.Python.Analysis.Types; using Microsoft.Python.Analysis.Values; using Microsoft.Python.Core; @@ -98,19 +99,29 @@ private PythonClassType CreateClass(ClassDefinition cd) { private void AddFunctionOrProperty(FunctionDefinition fd) { var declaringType = fd.Parent != null && _typeMap.TryGetValue(fd.Parent, out var t) ? t : null; - if (!TryAddProperty(fd, declaringType)) { - AddFunction(fd, declaringType); + var existing = _eval.LookupNameInScopes(fd.Name, LookupOptions.Local); + + switch (existing?.MemberType) { + case PythonMemberType.Method: + case PythonMemberType.Function: + case PythonMemberType.Property: + ReportRedefinedFunction(fd, existing as PythonType); + break; + } + + if (!TryAddProperty(fd, existing, declaringType)) { + AddFunction(fd, existing, declaringType); } } - private void AddFunction(FunctionDefinition fd, IPythonType declaringType) { - if (!(_eval.LookupNameInScopes(fd.Name, LookupOptions.Local) is PythonFunctionType existing)) { - existing = new PythonFunctionType(fd, declaringType, _eval.GetLocationOfName(fd)); + private void AddFunction(FunctionDefinition fd, IMember existing, IPythonType declaringType) { + if (!(existing is PythonFunctionType f)) { + f = new PythonFunctionType(fd, declaringType, _eval.GetLocationOfName(fd)); // The variable is transient (non-user declared) hence it does not have location. // Function type is tracking locations for references and renaming. - _eval.DeclareVariable(fd.Name, existing, VariableSource.Declaration); + _eval.DeclareVariable(fd.Name, f, VariableSource.Declaration); } - AddOverload(fd, existing, o => existing.AddOverload(o)); + AddOverload(fd, f, o => f.AddOverload(o)); } private void AddOverload(FunctionDefinition fd, IPythonClassMember function, Action addOverload) { @@ -148,31 +159,31 @@ private PythonFunctionOverload GetOverloadFromStub(FunctionDefinition node) { return null; } - private bool TryAddProperty(FunctionDefinition node, IPythonType declaringType) { + private bool TryAddProperty(FunctionDefinition node, IMember existing, IPythonType declaringType) { var dec = node.Decorators?.Decorators; var decorators = dec != null ? dec.ExcludeDefault().ToArray() : Array.Empty(); foreach (var d in decorators.OfType()) { switch (d.Name) { case @"property": - AddProperty(node, declaringType, false); + AddProperty(node, existing, declaringType, false); return true; case @"abstractproperty": - AddProperty(node, declaringType, true); + AddProperty(node, existing, declaringType, true); return true; } } return false; } - private void AddProperty(FunctionDefinition fd, IPythonType declaringType, bool isAbstract) { - if (!(_eval.LookupNameInScopes(fd.Name, LookupOptions.Local) is PythonPropertyType existing)) { - existing = new PythonPropertyType(fd, _eval.GetLocationOfName(fd), declaringType, isAbstract); + private void AddProperty(FunctionDefinition fd, IMember existing, IPythonType declaringType, bool isAbstract) { + if (!(existing is PythonPropertyType p)) { + p = new PythonPropertyType(fd, _eval.GetLocationOfName(fd), declaringType, isAbstract); // 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); + _eval.DeclareVariable(fd.Name, p, VariableSource.Declaration); } - AddOverload(fd, existing, o => existing.AddOverload(o)); + AddOverload(fd, p, o => p.AddOverload(o)); } private IMember GetMemberFromStub(string name) { @@ -202,6 +213,22 @@ private IMember GetMemberFromStub(string name) { return member; } + private void ReportRedefinedFunction(FunctionDefinition redefined, ILocatedMember existing) { + // get line number of existing function for diagnostic message + var existingLoc = existing.Definition; + var existingLine = existingLoc.Span.Start.Line; + + _eval.ReportDiagnostics( + _eval.Module.Uri, + new DiagnosticsEntry( + Resources.FunctionRedefined.FormatInvariant(existingLine), + // only highlight the redefined name + _eval.GetLocationInfo(redefined.NameExpression).Span, + ErrorCodes.FunctionRedefined, + Parsing.Severity.Error, + DiagnosticSource.Analysis)); + } + private static bool IsDeprecated(ClassDefinition cd) => cd.Decorators?.Decorators != null && IsDeprecated(cd.Decorators.Decorators); diff --git a/src/Analysis/Ast/Impl/Diagnostics/ErrorCodes.cs b/src/Analysis/Ast/Impl/Diagnostics/ErrorCodes.cs index 7a5f419c2..766fd5255 100644 --- a/src/Analysis/Ast/Impl/Diagnostics/ErrorCodes.cs +++ b/src/Analysis/Ast/Impl/Diagnostics/ErrorCodes.cs @@ -25,6 +25,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 FunctionRedefined = "function-redefined"; + public const string UnsupportedOperandType = "unsupported-operand-type"; public const string ReturnInInit = "return-in-init"; public const string TypingNewTypeArguments = "typing-newtype-arguments"; public const string TypingGenericArguments = "typing-generic-arguments"; diff --git a/src/Analysis/Ast/Impl/Resources.Designer.cs b/src/Analysis/Ast/Impl/Resources.Designer.cs index 511ddd1b9..c070c60f8 100644 --- a/src/Analysis/Ast/Impl/Resources.Designer.cs +++ b/src/Analysis/Ast/Impl/Resources.Designer.cs @@ -231,6 +231,15 @@ internal static string ExceptionGettingSearchPaths { } } + /// + /// Looks up a localized string similar to Function already defined at line {0}.. + /// + internal static string FunctionRedefined { + get { + return ResourceManager.GetString("FunctionRedefined", resourceCulture); + } + } + /// /// Looks up a localized string similar to Arguments to Generic must all be type parameters.. /// diff --git a/src/Analysis/Ast/Impl/Resources.resx b/src/Analysis/Ast/Impl/Resources.resx index b939ac1c8..dbf846772 100644 --- a/src/Analysis/Ast/Impl/Resources.resx +++ b/src/Analysis/Ast/Impl/Resources.resx @@ -189,6 +189,9 @@ Unable to determine analysis cache path. Exception: {0}. Using default '{1}'. + + Function already defined at line {0}. + The first argument to NewType must be a string, but it is of type '{0}'. diff --git a/src/Analysis/Ast/Test/LintRedefinedFunctionTests.cs b/src/Analysis/Ast/Test/LintRedefinedFunctionTests.cs new file mode 100644 index 000000000..0343256f8 --- /dev/null +++ b/src/Analysis/Ast/Test/LintRedefinedFunctionTests.cs @@ -0,0 +1,329 @@ +// 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.IO; +using System.Linq; +using System.Threading.Tasks; +using FluentAssertions; +using Microsoft.Python.Analysis.Analyzer; +using Microsoft.Python.Analysis.Diagnostics; +using Microsoft.Python.Analysis.Documents; +using Microsoft.Python.Analysis.Tests.FluentAssertions; +using Microsoft.Python.Core; +using Microsoft.Python.Parsing.Tests; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using TestUtilities; + +namespace Microsoft.Python.Analysis.Tests { + [TestClass] + public class LintRedefinedFunctionTests : AnalysisTestBase { + public TestContext TestContext { get; set; } + + [TestInitialize] + public void TestInitialize() + => TestEnvironmentImpl.TestInitialize($"{TestContext.FullyQualifiedTestClassName}.{TestContext.TestName}"); + + [TestCleanup] + public void Cleanup() => TestEnvironmentImpl.TestCleanup(); + + [TestMethod, Priority(0)] + public async Task BasicRedefinedFuncTest() { + const string code = @" +def hello(): + pass + +def hello(test): + print(test) +"; + var analysis = await GetAnalysisAsync(code); + analysis.Diagnostics.Should().HaveCount(1); + + var diagnostic = analysis.Diagnostics.ElementAt(0); + diagnostic.ErrorCode.Should().Be(ErrorCodes.FunctionRedefined); + diagnostic.SourceSpan.Should().Be(5, 5, 5, 10); + diagnostic.Message.Should().Be(Resources.FunctionRedefined.FormatInvariant(2)); + } + + [TestMethod, Priority(0)] + public async Task RedefinedFuncInClassTest() { + const string code = @" +class Test: + def hello(): + pass + + def hello(test): + print(test) +"; + var analysis = await GetAnalysisAsync(code); + analysis.Diagnostics.Should().HaveCount(1); + + var diagnostic = analysis.Diagnostics.ElementAt(0); + diagnostic.ErrorCode.Should().Be(ErrorCodes.FunctionRedefined); + diagnostic.SourceSpan.Should().Be(6, 9, 6, 14); + diagnostic.Message.Should().Be(Resources.FunctionRedefined.FormatInvariant(3)); + } + + [TestMethod, Priority(0)] + public async Task ValidRedefinitionsNoErrors() { + const string code = @" +def foo(): + pass + +bar = foo + +def bar(): + pass +"; + var analysis = await GetAnalysisAsync(code); + analysis.Diagnostics.Should().HaveCount(0); + } + + [TestMethod, Priority(0)] + public async Task NestedFunctionValid() { + const string code = @" +def foo(): + def foo(): + return 123 + return foo() + +foo() +"; + var analysis = await GetAnalysisAsync(code); + analysis.Diagnostics.Should().HaveCount(0); + } + + [TestMethod, Priority(0)] + public async Task RedefineFunctionInNestedFunction() { + const string code = @" +def foo(): + def foo(): + return 123 + + def foo(): + return 213 + + return foo() + +foo() +"; + var analysis = await GetAnalysisAsync(code); + analysis.Diagnostics.Should().HaveCount(1); + + var diagnostic = analysis.Diagnostics.ElementAt(0); + diagnostic.ErrorCode.Should().Be(ErrorCodes.FunctionRedefined); + diagnostic.SourceSpan.Should().Be(6, 9, 6, 12); + diagnostic.Message.Should().Be(Resources.FunctionRedefined.FormatInvariant(3)); + } + + [TestMethod, Priority(0)] + public async Task SameFunctionInNestedClassDifferentScope() { + const string code = @" +class tmp: + def foo(self): + return 123 + + class tmp1: + def foo(self): + return 213 +"; + var analysis = await GetAnalysisAsync(code); + analysis.Diagnostics.Should().HaveCount(0); + } + + [TestMethod, Priority(0)] + public async Task RedefineFunctionInNestedClass() { + const string code = @" +class tmp: + def foo(self): + return 123 + + class tmp1: + def foo(self): + return 213 + + def foo(self): + return 4784 + +"; + var analysis = await GetAnalysisAsync(code); + analysis.Diagnostics.Should().HaveCount(1); + + var diagnostic = analysis.Diagnostics.ElementAt(0); + diagnostic.ErrorCode.Should().Be(ErrorCodes.FunctionRedefined); + diagnostic.SourceSpan.Should().Be(10, 13, 10, 16); + diagnostic.Message.Should().Be(Resources.FunctionRedefined.FormatInvariant(7)); + } + + + /// + /// Finish when can handle conditional functions declarations (if ever) + /// + /// + [Ignore] + [TestMethod, Priority(0)] + public async Task ValidConditionalNoErrors() { + const string code = @" +if 1 == 1: + def a(): + pass +else: + def a(): + pass +"; + var analysis = await GetAnalysisAsync(code); + analysis.Diagnostics.Should().HaveCount(0); + } + + [TestMethod, Priority(0)] + public async Task RedefinedFuncsOtherModules() { + var module1Code = @" +def hello(): + pass + +def hello2(): + pass +"; + + var appCode = @" +from module1 import * + +def hello(): + pass +"; + + var module1Uri = TestData.GetTestSpecificUri("module1.py"); + var appUri = TestData.GetTestSpecificUri("app.py"); + + var root = Path.GetDirectoryName(appUri.AbsolutePath); + await CreateServicesAsync(root, PythonVersions.LatestAvailable3X); + var rdt = Services.GetService(); + var analyzer = Services.GetService(); + + rdt.OpenDocument(module1Uri, module1Code); + + var app = rdt.OpenDocument(appUri, appCode); + await analyzer.WaitForCompleteAnalysisAsync(); + var analysis = await app.GetAnalysisAsync(-1); + analysis.Diagnostics.Should().BeEmpty(); + } + + [TestMethod, Priority(0)] + public async Task RedefinedFuncsOtherModulesNamedImport() { + var module1Code = @" +def hello(): + pass + +def hello2(): + pass +"; + + var appCode = @" +from module1 import hello + +def hello(): + pass +"; + + var module1Uri = TestData.GetTestSpecificUri("module1.py"); + var appUri = TestData.GetTestSpecificUri("app.py"); + + var root = Path.GetDirectoryName(appUri.AbsolutePath); + await CreateServicesAsync(root, PythonVersions.LatestAvailable3X); + var rdt = Services.GetService(); + var analyzer = Services.GetService(); + + rdt.OpenDocument(module1Uri, module1Code); + + var app = rdt.OpenDocument(appUri, appCode); + await analyzer.WaitForCompleteAnalysisAsync(); + var analysis = await app.GetAnalysisAsync(-1); + analysis.Diagnostics.Should().BeEmpty(); + } + + [TestMethod, Priority(0)] + public async Task RedefinedFuncsOtherModulesRenamedImport() { + var module1Code = @" +def hello(): + pass + +def hello2(): + pass +"; + + var appCode = @" +from module1 import hello2 as hello + +def hello(): + pass +"; + + var module1Uri = TestData.GetTestSpecificUri("module1.py"); + var appUri = TestData.GetTestSpecificUri("app.py"); + + var root = Path.GetDirectoryName(appUri.AbsolutePath); + await CreateServicesAsync(root, PythonVersions.LatestAvailable3X); + var rdt = Services.GetService(); + var analyzer = Services.GetService(); + + rdt.OpenDocument(module1Uri, module1Code); + + var app = rdt.OpenDocument(appUri, appCode); + await analyzer.WaitForCompleteAnalysisAsync(); + var analysis = await app.GetAnalysisAsync(-1); + analysis.Diagnostics.Should().BeEmpty(); + } + + [TestMethod, Priority(0)] + public async Task RedefinedFuncsOneIsProperty() { + const string code = @" +class tmp: + def foo(self): + return 123 + + @property + def foo(self): + return 123 +"; + var analysis = await GetAnalysisAsync(code); + analysis.Diagnostics.Should().HaveCount(1); + + var diagnostic = analysis.Diagnostics.ElementAt(0); + diagnostic.ErrorCode.Should().Be(ErrorCodes.FunctionRedefined); + diagnostic.SourceSpan.Should().Be(7, 9, 7, 12); + diagnostic.Message.Should().Be(Resources.FunctionRedefined.FormatInvariant(3)); + } + + [TestMethod, Priority(0)] + public async Task RedefinedFuncsBothAreProperties() { + const string code = @" +class tmp: + @property + def foo(self): + return 123 + + @property + def foo(self): + return 123 +"; + var analysis = await GetAnalysisAsync(code); + analysis.Diagnostics.Should().HaveCount(1); + + var diagnostic = analysis.Diagnostics.ElementAt(0); + diagnostic.ErrorCode.Should().Be(ErrorCodes.FunctionRedefined); + diagnostic.SourceSpan.Should().Be(8, 9, 8, 12); + diagnostic.Message.Should().Be(Resources.FunctionRedefined.FormatInvariant(4)); + } + } +}