diff --git a/src/Analysis/Ast/Impl/Analyzer/Symbols/SymbolCollector.cs b/src/Analysis/Ast/Impl/Analyzer/Symbols/SymbolCollector.cs index 2a70d6b56..a3f489cf7 100644 --- a/src/Analysis/Ast/Impl/Analyzer/Symbols/SymbolCollector.cs +++ b/src/Analysis/Ast/Impl/Analyzer/Symbols/SymbolCollector.cs @@ -99,29 +99,19 @@ private PythonClassType CreateClass(ClassDefinition cd) { private void AddFunctionOrProperty(FunctionDefinition fd) { var declaringType = fd.Parent != null && _typeMap.TryGetValue(fd.Parent, out var t) ? t : null; - 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); + if (!TryAddProperty(fd, declaringType)) { + AddFunction(fd, declaringType); } } - private void AddFunction(FunctionDefinition fd, IMember existing, IPythonType declaringType) { - if (!(existing is PythonFunctionType f)) { - f = new PythonFunctionType(fd, declaringType, _eval.GetLocationOfName(fd)); + 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)); // 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, f, VariableSource.Declaration); + _eval.DeclareVariable(fd.Name, existing, VariableSource.Declaration); } - AddOverload(fd, f, o => f.AddOverload(o)); + AddOverload(fd, existing, o => existing.AddOverload(o)); } private void AddOverload(FunctionDefinition fd, IPythonClassMember function, Action addOverload) { @@ -159,31 +149,31 @@ private PythonFunctionOverload GetOverloadFromStub(FunctionDefinition node) { return null; } - private bool TryAddProperty(FunctionDefinition node, IMember existing, IPythonType declaringType) { + private bool TryAddProperty(FunctionDefinition node, 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, existing, declaringType, false); + AddProperty(node, declaringType, false); return true; case @"abstractproperty": - AddProperty(node, existing, declaringType, true); + AddProperty(node, declaringType, true); return true; } } return false; } - 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); + 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); // 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, p, VariableSource.Declaration); + _eval.DeclareVariable(fd.Name, existing, VariableSource.Declaration); } - AddOverload(fd, p, o => p.AddOverload(o)); + AddOverload(fd, existing, o => existing.AddOverload(o)); } private IMember GetMemberFromStub(string name) { @@ -213,22 +203,6 @@ 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 fe679ae1e..2642df805 100644 --- a/src/Analysis/Ast/Impl/Diagnostics/ErrorCodes.cs +++ b/src/Analysis/Ast/Impl/Diagnostics/ErrorCodes.cs @@ -25,8 +25,6 @@ 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 TypingTypeVarArguments = "typing-typevar-arguments"; public const string TypingNewTypeArguments = "typing-newtype-arguments"; diff --git a/src/Analysis/Ast/Impl/Resources.Designer.cs b/src/Analysis/Ast/Impl/Resources.Designer.cs index 1ad4308d4..88c660d70 100644 --- a/src/Analysis/Ast/Impl/Resources.Designer.cs +++ b/src/Analysis/Ast/Impl/Resources.Designer.cs @@ -231,15 +231,6 @@ 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 f96c3346a..95fdc4a5c 100644 --- a/src/Analysis/Ast/Impl/Resources.resx +++ b/src/Analysis/Ast/Impl/Resources.resx @@ -195,9 +195,6 @@ A single constraint to TypeVar is not allowed. - - 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 deleted file mode 100644 index 0343256f8..000000000 --- a/src/Analysis/Ast/Test/LintRedefinedFunctionTests.cs +++ /dev/null @@ -1,329 +0,0 @@ -// 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)); - } - } -}