From a3e505ddf695fb87a9e458a2d54b5324f39c8102 Mon Sep 17 00:00:00 2001 From: Cameron Trando Date: Tue, 2 Jul 2019 14:23:51 -0700 Subject: [PATCH 01/14] Adding NoQA and tests --- .../Analyzer/Evaluation/ExpressionEval.cs | 3 +- .../Ast/Impl/Analyzer/PythonAnalyzer.cs | 2 +- .../Ast/Impl/Diagnostics/DiagnosticsEntry.cs | 16 +++ .../Ast/Impl/Linting/LinterAggregator.cs | 5 +- src/Analysis/Ast/Impl/Modules/PythonModule.cs | 20 +++- src/Analysis/Ast/Test/LintNoQATests.cs | 97 +++++++++++++++++++ 6 files changed, 138 insertions(+), 5 deletions(-) create mode 100644 src/Analysis/Ast/Test/LintNoQATests.cs diff --git a/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.cs b/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.cs index 55aa4296e..4e0b7d609 100644 --- a/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.cs +++ b/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.cs @@ -16,6 +16,7 @@ using System; using System.Collections.Generic; using System.Diagnostics; +using System.Reflection; using Microsoft.Python.Analysis.Analyzer.Symbols; using Microsoft.Python.Analysis.Diagnostics; using Microsoft.Python.Analysis.Modules; @@ -108,7 +109,7 @@ public Location GetLocationOfName(Node node) { public void ReportDiagnostics(Uri documentUri, DiagnosticsEntry entry) { // Do not add if module is library, etc. Only handle user code. - if (Module.ModuleType == ModuleType.User) { + if (entry.ShouldReport(Module)) { lock (_lock) { _diagnostics.Add(entry); } diff --git a/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs b/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs index 0f3c037fc..66165504b 100644 --- a/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs +++ b/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs @@ -174,7 +174,7 @@ public IReadOnlyList LintModule(IPythonModule module) { // Linter always runs no matter of the option since it looks up variables // which also enumerates and updates variable references for find all // references and rename operations. - var result = new LinterAggregator().Lint(module.Analysis, _services); + var result = new LinterAggregator().Lint(module, _services); var optionsProvider = _services.GetService(); return optionsProvider?.Options?.LintingEnabled == false ? Array.Empty() : result; diff --git a/src/Analysis/Ast/Impl/Diagnostics/DiagnosticsEntry.cs b/src/Analysis/Ast/Impl/Diagnostics/DiagnosticsEntry.cs index 25b174e69..e1b2471cf 100644 --- a/src/Analysis/Ast/Impl/Diagnostics/DiagnosticsEntry.cs +++ b/src/Analysis/Ast/Impl/Diagnostics/DiagnosticsEntry.cs @@ -13,6 +13,8 @@ // See the Apache Version 2.0 License for specific language governing // permissions and limitations under the License. +using Microsoft.Python.Analysis.Modules; +using Microsoft.Python.Analysis.Types; using Microsoft.Python.Core.Text; using Microsoft.Python.Parsing; @@ -51,6 +53,20 @@ public DiagnosticsEntry(string message, SourceSpan span, string errorCode, Sever /// public DiagnosticSource Source { get; } + public bool ShouldReport(IPythonModule module) { + // module should always be a user written python module + if (module.ModuleType != ModuleType.User || !(module is PythonModule pythonModule)) { + return false; + } + + string line = pythonModule.GetLine(SourceSpan.Start.Line); + if (line.Contains("#noqa")) { + return false; + } + + return true; + } + public override bool Equals(object obj) { if (!(obj is DiagnosticsEntry e)) { return false; diff --git a/src/Analysis/Ast/Impl/Linting/LinterAggregator.cs b/src/Analysis/Ast/Impl/Linting/LinterAggregator.cs index 948bd55f3..915ddf4a5 100644 --- a/src/Analysis/Ast/Impl/Linting/LinterAggregator.cs +++ b/src/Analysis/Ast/Impl/Linting/LinterAggregator.cs @@ -17,6 +17,7 @@ using System.Linq; using Microsoft.Python.Analysis.Diagnostics; using Microsoft.Python.Analysis.Linting.UndefinedVariables; +using Microsoft.Python.Analysis.Types; using Microsoft.Python.Core; namespace Microsoft.Python.Analysis.Linting { @@ -27,7 +28,7 @@ public LinterAggregator() { // TODO: develop mechanism for dynamic and external linter discovery. _linters.Add(new UndefinedVariablesLinter()); } - public IReadOnlyList Lint(IDocumentAnalysis analysis, IServiceContainer services) - => _linters.SelectMany(l => l.Lint(analysis, services)).ToArray(); + public IReadOnlyList Lint(IPythonModule module, IServiceContainer services) + => _linters.SelectMany(l => l.Lint(module.Analysis, services)).Where(d => d.ShouldReport(module)).ToArray(); } } diff --git a/src/Analysis/Ast/Impl/Modules/PythonModule.cs b/src/Analysis/Ast/Impl/Modules/PythonModule.cs index 81664250d..5d641ba51 100644 --- a/src/Analysis/Ast/Impl/Modules/PythonModule.cs +++ b/src/Analysis/Ast/Impl/Modules/PythonModule.cs @@ -243,6 +243,25 @@ public string Content { } } + /// + /// Returns the string line corresponding to the given location + /// + /// The Ast + /// The start location + public string GetLine(int line) { + StringBuilder sb = new StringBuilder(); + SourceLocation source = new SourceLocation(line, 1); + string content = Content; + var index = this.GetAst().LocationToIndex(source); + + if (!content.IsNullOrEmpty()) { + for (; index < content.Length && content[index] != '\n' && content[index] != '\r'; index++) { + sb.Append(content[index]); + } + } + return sb.ToString(); + } + #endregion #region Parsing @@ -376,7 +395,6 @@ private void Parse(CancellationToken cancellationToken) { private class CollectingErrorSink : ErrorSink { private readonly List _diagnostics = new List(); - public IReadOnlyList Diagnostics => _diagnostics; public override void Add(string message, SourceSpan span, int errorCode, Severity severity) => _diagnostics.Add(new DiagnosticsEntry(message, span, $"parser-{errorCode}", severity, DiagnosticSource.Parser)); diff --git a/src/Analysis/Ast/Test/LintNoQATests.cs b/src/Analysis/Ast/Test/LintNoQATests.cs new file mode 100644 index 000000000..49d1f42fd --- /dev/null +++ b/src/Analysis/Ast/Test/LintNoQATests.cs @@ -0,0 +1,97 @@ +using System.Collections.Generic; +using System.Threading.Tasks; +using FluentAssertions; +using Microsoft.Python.Analysis.Analyzer; +using Microsoft.Python.Analysis.Core.Interpreter; +using Microsoft.Python.Analysis.Diagnostics; +using Microsoft.Python.Analysis.Tests.FluentAssertions; +using Microsoft.Python.Parsing.Tests; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using TestUtilities; + + +namespace Microsoft.Python.Analysis.Tests { + + [TestClass] + public class LintNoQATests : AnalysisTestBase { + + public const string GenericSetup = @" +from typing import Generic, TypeVar +T = TypeVar('T', int, str) +T1 = TypeVar('T1', int, str) + +_X = TypeVar('_X', str, int) +_T = _X +"; + + public TestContext TestContext { get; set; } + + [TestInitialize] + public void TestInitialize() + => TestEnvironmentImpl.TestInitialize($"{TestContext.FullyQualifiedTestClassName}.{TestContext.TestName}"); + + [TestCleanup] + public void Cleanup() => TestEnvironmentImpl.TestCleanup(); + + [DataRow("x = Generic[T, str] #noqa")] + [DataRow("x = Generic[T, T] #noqa ")] + [DataTestMethod, Priority(0)] + public async Task IgnoreGenerics(string decl) { + string code = GenericSetup + decl; + + var analysis = await GetAnalysisAsync(code); + analysis.Diagnostics.Should().HaveCount(0); + } + + [DataRow("x = 'str' + 5 #noqa")] + [DataRow("x = float(1) * 'str' #noqa")] + [DataTestMethod, Priority(0)] + public async Task IgnoreBadBinaryOp(string code) { + var analysis = await GetAnalysisAsync(code); + analysis.Diagnostics.Should().HaveCount(0); + } + + [DataRow("x = y #noqa")] + [DataRow("x = z + 2 #noqa")] + [DataTestMethod, Priority(0)] + public async Task IgnoreUndefinedVar(string code) { + var d = await LintAsync(code); + d.Should().HaveCount(0); + } + + [DataRow(@" +def hello(a, b, c): + pass + +h = hello(1, 2) +")] + [DataRow(@" +class Hello: + def tmp(self, a): + pass + +h = Hello() +t = h() +")] + [DataTestMethod, Priority(0)] + public async Task IgnoreMissingParameter(string code) { + var d = await LintAsync(code); + d.Should().HaveCount(0); + } + + [TestMethod, Priority(0)] + public async Task IgnoreMissingImport() { + string code = @" +from django.contrib.auth.models import User #noqa +"; + var analysis = await GetAnalysisAsync(code); + analysis.Diagnostics.Should().HaveCount(0); + } + + private async Task> LintAsync(string code, InterpreterConfiguration configuration = null) { + var analysis = await GetAnalysisAsync(code, configuration ?? PythonVersions.LatestAvailable3X); + var a = Services.GetService(); + return a.LintModule(analysis.Document); + } + } +} From 6d4e7df0c41b39aaea708a2bb0158ab2f0de22a3 Mon Sep 17 00:00:00 2001 From: Cameron Trando Date: Tue, 2 Jul 2019 14:37:00 -0700 Subject: [PATCH 02/14] Minor changes --- src/Analysis/Ast/Impl/Modules/PythonModule.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Analysis/Ast/Impl/Modules/PythonModule.cs b/src/Analysis/Ast/Impl/Modules/PythonModule.cs index 5d641ba51..e71b82c59 100644 --- a/src/Analysis/Ast/Impl/Modules/PythonModule.cs +++ b/src/Analysis/Ast/Impl/Modules/PythonModule.cs @@ -246,8 +246,7 @@ public string Content { /// /// Returns the string line corresponding to the given location /// - /// The Ast - /// The start location + /// The line number public string GetLine(int line) { StringBuilder sb = new StringBuilder(); SourceLocation source = new SourceLocation(line, 1); @@ -395,6 +394,7 @@ private void Parse(CancellationToken cancellationToken) { private class CollectingErrorSink : ErrorSink { private readonly List _diagnostics = new List(); + public IReadOnlyList Diagnostics => _diagnostics; public override void Add(string message, SourceSpan span, int errorCode, Severity severity) => _diagnostics.Add(new DiagnosticsEntry(message, span, $"parser-{errorCode}", severity, DiagnosticSource.Parser)); From c7439d5c53c04341ab2eafab96b39868bb38a793 Mon Sep 17 00:00:00 2001 From: Cameron Trando Date: Tue, 2 Jul 2019 14:57:17 -0700 Subject: [PATCH 03/14] Checking for noqa in the comment, not for #noqa --- .../Ast/Impl/Analyzer/Evaluation/ExpressionEval.cs | 2 -- .../Ast/Impl/Diagnostics/DiagnosticsEntry.cs | 10 ++++++++-- src/Analysis/Ast/Test/LintNoQATests.cs | 13 ++++++++++++- 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.cs b/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.cs index 4e0b7d609..1375d1f10 100644 --- a/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.cs +++ b/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.cs @@ -16,12 +16,10 @@ using System; using System.Collections.Generic; using System.Diagnostics; -using System.Reflection; using Microsoft.Python.Analysis.Analyzer.Symbols; using Microsoft.Python.Analysis.Diagnostics; using Microsoft.Python.Analysis.Modules; using Microsoft.Python.Analysis.Types; -using Microsoft.Python.Analysis.Utilities; using Microsoft.Python.Analysis.Values; using Microsoft.Python.Core; using Microsoft.Python.Core.Disposables; diff --git a/src/Analysis/Ast/Impl/Diagnostics/DiagnosticsEntry.cs b/src/Analysis/Ast/Impl/Diagnostics/DiagnosticsEntry.cs index e1b2471cf..7168c4282 100644 --- a/src/Analysis/Ast/Impl/Diagnostics/DiagnosticsEntry.cs +++ b/src/Analysis/Ast/Impl/Diagnostics/DiagnosticsEntry.cs @@ -60,8 +60,14 @@ public bool ShouldReport(IPythonModule module) { } string line = pythonModule.GetLine(SourceSpan.Start.Line); - if (line.Contains("#noqa")) { - return false; + + // find if #noqa is in the string + int commentPos = line.IndexOf("#"); + if (commentPos >= 0) { + var comment = line.Substring(commentPos + 1); + if (comment.Contains("noqa")) { + return false; + } } return true; diff --git a/src/Analysis/Ast/Test/LintNoQATests.cs b/src/Analysis/Ast/Test/LintNoQATests.cs index 49d1f42fd..a31576996 100644 --- a/src/Analysis/Ast/Test/LintNoQATests.cs +++ b/src/Analysis/Ast/Test/LintNoQATests.cs @@ -51,6 +51,17 @@ public async Task IgnoreBadBinaryOp(string code) { analysis.Diagnostics.Should().HaveCount(0); } + [DataRow("x = 'str' + 5 # noqa")] + [DataRow("x = float(1) * 'str' # noqa")] + [DataTestMethod, Priority(0)] + public async Task IgnoreNoQAWithSpace(string code) { + var analysis = await GetAnalysisAsync(code); + analysis.Diagnostics.Should().HaveCount(0); + } + + + + [DataRow("x = y #noqa")] [DataRow("x = z + 2 #noqa")] [DataTestMethod, Priority(0)] @@ -82,7 +93,7 @@ public async Task IgnoreMissingParameter(string code) { [TestMethod, Priority(0)] public async Task IgnoreMissingImport() { string code = @" -from django.contrib.auth.models import User #noqa +from fake_module import User #noqa "; var analysis = await GetAnalysisAsync(code); analysis.Diagnostics.Should().HaveCount(0); From df5d68286eb4430f60df26ed33839e49459f5c33 Mon Sep 17 00:00:00 2001 From: Cameron Trando Date: Wed, 3 Jul 2019 13:03:20 -0700 Subject: [PATCH 04/14] Moved method to extension and removed test that did not contribute anything --- .../Impl/Extensions/PythonModuleExtensions.cs | 20 ++++++++++++++++++ src/Analysis/Ast/Impl/Modules/PythonModule.cs | 21 +------------------ src/Analysis/Ast/Test/LintNoQATests.cs | 20 ------------------ 3 files changed, 21 insertions(+), 40 deletions(-) diff --git a/src/Analysis/Ast/Impl/Extensions/PythonModuleExtensions.cs b/src/Analysis/Ast/Impl/Extensions/PythonModuleExtensions.cs index c33b4620f..85f903062 100644 --- a/src/Analysis/Ast/Impl/Extensions/PythonModuleExtensions.cs +++ b/src/Analysis/Ast/Impl/Extensions/PythonModuleExtensions.cs @@ -15,6 +15,8 @@ using Microsoft.Python.Analysis.Modules; using Microsoft.Python.Analysis.Types; +using Microsoft.Python.Core; +using Microsoft.Python.Core.Text; using Microsoft.Python.Parsing.Ast; namespace Microsoft.Python.Analysis { @@ -26,5 +28,23 @@ internal static T GetAstNode(this IPythonModule module, object o) where T : N => ((IAstNodeContainer)module).GetAstNode(o); internal static void AddAstNode(this IPythonModule module, object o, Node n) => ((IAstNodeContainer)module).AddAstNode(o, n); + + /// + /// Returns the string line corresponding to the given location + /// + /// The line number + internal static string GetLine(this PythonModule module, int line) { + SourceLocation source = new SourceLocation(line, 1); + string content = module.Content; + var start = GetAst(module).LocationToIndex(source); + var length = 0; + + if (!content.IsNullOrEmpty()) { + while(start + length < content.Length && content[start + length] != '\n' && content[start + length] != '\r') { + length++; + } + } + return content.Substring(start, length); + } } } diff --git a/src/Analysis/Ast/Impl/Modules/PythonModule.cs b/src/Analysis/Ast/Impl/Modules/PythonModule.cs index e71b82c59..25c507bd0 100644 --- a/src/Analysis/Ast/Impl/Modules/PythonModule.cs +++ b/src/Analysis/Ast/Impl/Modules/PythonModule.cs @@ -241,26 +241,7 @@ public string Content { return _buffer.Text; } } - } - - /// - /// Returns the string line corresponding to the given location - /// - /// The line number - public string GetLine(int line) { - StringBuilder sb = new StringBuilder(); - SourceLocation source = new SourceLocation(line, 1); - string content = Content; - var index = this.GetAst().LocationToIndex(source); - - if (!content.IsNullOrEmpty()) { - for (; index < content.Length && content[index] != '\n' && content[index] != '\r'; index++) { - sb.Append(content[index]); - } - } - return sb.ToString(); - } - + } #endregion #region Parsing diff --git a/src/Analysis/Ast/Test/LintNoQATests.cs b/src/Analysis/Ast/Test/LintNoQATests.cs index a31576996..5ad5556a2 100644 --- a/src/Analysis/Ast/Test/LintNoQATests.cs +++ b/src/Analysis/Ast/Test/LintNoQATests.cs @@ -70,26 +70,6 @@ public async Task IgnoreUndefinedVar(string code) { d.Should().HaveCount(0); } - [DataRow(@" -def hello(a, b, c): - pass - -h = hello(1, 2) -")] - [DataRow(@" -class Hello: - def tmp(self, a): - pass - -h = Hello() -t = h() -")] - [DataTestMethod, Priority(0)] - public async Task IgnoreMissingParameter(string code) { - var d = await LintAsync(code); - d.Should().HaveCount(0); - } - [TestMethod, Priority(0)] public async Task IgnoreMissingImport() { string code = @" From e7dd636f8ed101ac9ee0855d933718047eca529f Mon Sep 17 00:00:00 2001 From: Cameron Trando Date: Wed, 3 Jul 2019 15:04:46 -0700 Subject: [PATCH 05/14] Changing loop style, handling null better --- .../Ast/Impl/Extensions/PythonModuleExtensions.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Analysis/Ast/Impl/Extensions/PythonModuleExtensions.cs b/src/Analysis/Ast/Impl/Extensions/PythonModuleExtensions.cs index 85f903062..c89368680 100644 --- a/src/Analysis/Ast/Impl/Extensions/PythonModuleExtensions.cs +++ b/src/Analysis/Ast/Impl/Extensions/PythonModuleExtensions.cs @@ -37,14 +37,14 @@ internal static string GetLine(this PythonModule module, int line) { SourceLocation source = new SourceLocation(line, 1); string content = module.Content; var start = GetAst(module).LocationToIndex(source); - var length = 0; + var end = start; - if (!content.IsNullOrEmpty()) { - while(start + length < content.Length && content[start + length] != '\n' && content[start + length] != '\r') { - length++; - } + if (string.IsNullOrEmpty(content)) { + return content; } - return content.Substring(start, length); + + for (; end < content.Length && content[end] != '\n' && content[end] != '\r'; end++); + return content.Substring(start, end - start); } } } From cd707ca365a67a7748db5a7ff7b836999e6a43e1 Mon Sep 17 00:00:00 2001 From: Cameron Trando Date: Tue, 9 Jul 2019 10:54:29 -0700 Subject: [PATCH 06/14] Shifting logic around --- .../Ast/Impl/Diagnostics/DiagnosticsEntry.cs | 11 ++-------- .../Ast/Impl/Extensions/AstExtensions.cs | 21 ++++++++++++++++--- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/src/Analysis/Ast/Impl/Diagnostics/DiagnosticsEntry.cs b/src/Analysis/Ast/Impl/Diagnostics/DiagnosticsEntry.cs index 7168c4282..3917b4a57 100644 --- a/src/Analysis/Ast/Impl/Diagnostics/DiagnosticsEntry.cs +++ b/src/Analysis/Ast/Impl/Diagnostics/DiagnosticsEntry.cs @@ -59,15 +59,8 @@ public bool ShouldReport(IPythonModule module) { return false; } - string line = pythonModule.GetLine(SourceSpan.Start.Line); - - // find if #noqa is in the string - int commentPos = line.IndexOf("#"); - if (commentPos >= 0) { - var comment = line.Substring(commentPos + 1); - if (comment.Contains("noqa")) { - return false; - } + if(!module.GetAst().HasComment(pythonModule, "noqa", SourceSpan.Start.Line)) { + return false; } return true; diff --git a/src/Analysis/Ast/Impl/Extensions/AstExtensions.cs b/src/Analysis/Ast/Impl/Extensions/AstExtensions.cs index a1d2b5d8f..a5fe42e78 100644 --- a/src/Analysis/Ast/Impl/Extensions/AstExtensions.cs +++ b/src/Analysis/Ast/Impl/Extensions/AstExtensions.cs @@ -17,16 +17,17 @@ using System.Diagnostics; using System.Linq; using Microsoft.Python.Analysis.Analyzer.Expressions; +using Microsoft.Python.Analysis.Modules; using Microsoft.Python.Core.Text; using Microsoft.Python.Parsing; using Microsoft.Python.Parsing.Ast; namespace Microsoft.Python.Analysis { public static class AstExtensions { - public static Expression FindExpression(this PythonAst ast, int index, FindExpressionOptions options) + public static Expression FindExpression(this PythonAst ast, int index, FindExpressionOptions options) => new ExpressionFinder(ast, options).GetExpression(index) as Expression; - public static Expression FindExpression(this PythonAst ast, SourceLocation location, FindExpressionOptions options) + public static Expression FindExpression(this PythonAst ast, SourceLocation location, FindExpressionOptions options) => new ExpressionFinder(ast, options).GetExpression(location) as Expression; public static string GetDocumentation(this ScopeStatement node) { @@ -60,9 +61,23 @@ public static bool IsInsideComment(this PythonAst ast, SourceLocation location) return ast.CommentLocations[match].Column < location.Column; } + internal static bool HasComment(this PythonAst ast, PythonModule module, string comment, int lineNum) { + var match = Array.BinarySearch(ast.CommentLocations, new SourceLocation(1, lineNum)); + + // line is not a comment + if (match < 0 || match >= ast.CommentLocations.Length) { + return false; + } + + var commentLoc = ast.CommentLocations[match]; + string line = module.GetLine(commentLoc.Line); + + return line.Contains(comment); + } + public static bool IsInsideString(this PythonAst ast, SourceLocation location) { var index = ast.LocationToIndex(location); - return ast.FindExpression(index, new FindExpressionOptions {Literals = true}) != null; + return ast.FindExpression(index, new FindExpressionOptions { Literals = true }) != null; } public static bool IsInParameter(this FunctionDefinition fd, PythonAst tree, SourceLocation location) { From 9bec166b1f77002b768115aba65752f15d224320 Mon Sep 17 00:00:00 2001 From: Cameron Trando Date: Wed, 10 Jul 2019 17:11:44 -0700 Subject: [PATCH 07/14] Doing binary search correctly on comment locations and adding test to make sure noqa works fine if it's not in a comment --- .../Ast/Impl/Diagnostics/DiagnosticsEntry.cs | 2 +- .../Ast/Impl/Extensions/AstExtensions.cs | 15 ++++++---- src/Analysis/Ast/Test/LintNoQATests.cs | 29 ++++++++++++++----- 3 files changed, 33 insertions(+), 13 deletions(-) diff --git a/src/Analysis/Ast/Impl/Diagnostics/DiagnosticsEntry.cs b/src/Analysis/Ast/Impl/Diagnostics/DiagnosticsEntry.cs index 3917b4a57..6b14cb53f 100644 --- a/src/Analysis/Ast/Impl/Diagnostics/DiagnosticsEntry.cs +++ b/src/Analysis/Ast/Impl/Diagnostics/DiagnosticsEntry.cs @@ -59,7 +59,7 @@ public bool ShouldReport(IPythonModule module) { return false; } - if(!module.GetAst().HasComment(pythonModule, "noqa", SourceSpan.Start.Line)) { + if (module.GetAst().HasComment(pythonModule, "noqa", SourceSpan.Start.Line)) { return false; } diff --git a/src/Analysis/Ast/Impl/Extensions/AstExtensions.cs b/src/Analysis/Ast/Impl/Extensions/AstExtensions.cs index a5fe42e78..4510a0513 100644 --- a/src/Analysis/Ast/Impl/Extensions/AstExtensions.cs +++ b/src/Analysis/Ast/Impl/Extensions/AstExtensions.cs @@ -62,17 +62,22 @@ public static bool IsInsideComment(this PythonAst ast, SourceLocation location) } internal static bool HasComment(this PythonAst ast, PythonModule module, string comment, int lineNum) { - var match = Array.BinarySearch(ast.CommentLocations, new SourceLocation(1, lineNum)); + var commentLines = ast.CommentLocations.Select(c => c.Line).ToArray(); + var match = Array.BinarySearch(commentLines, lineNum); // line is not a comment - if (match < 0 || match >= ast.CommentLocations.Length) { + if (match < 0 || match >= commentLines.Length) { return false; } - var commentLoc = ast.CommentLocations[match]; - string line = module.GetLine(commentLoc.Line); + string line = module.GetLine(lineNum); - return line.Contains(comment); + int commentPos = line.IndexOf('#'); + if (commentPos < 0) { + return false; + } + + return line.Substring(commentPos + 1).Contains(comment); } public static bool IsInsideString(this PythonAst ast, SourceLocation location) { diff --git a/src/Analysis/Ast/Test/LintNoQATests.cs b/src/Analysis/Ast/Test/LintNoQATests.cs index 5ad5556a2..def191ebe 100644 --- a/src/Analysis/Ast/Test/LintNoQATests.cs +++ b/src/Analysis/Ast/Test/LintNoQATests.cs @@ -1,14 +1,17 @@ using System.Collections.Generic; +using System.Linq; using System.Threading.Tasks; using FluentAssertions; using Microsoft.Python.Analysis.Analyzer; using Microsoft.Python.Analysis.Core.Interpreter; using Microsoft.Python.Analysis.Diagnostics; +using Microsoft.Python.Core; using Microsoft.Python.Analysis.Tests.FluentAssertions; +using Microsoft.Python.Parsing; using Microsoft.Python.Parsing.Tests; using Microsoft.VisualStudio.TestTools.UnitTesting; using TestUtilities; - +using ErrorCodes = Microsoft.Python.Analysis.Diagnostics.ErrorCodes; namespace Microsoft.Python.Analysis.Tests { @@ -40,7 +43,7 @@ public async Task IgnoreGenerics(string decl) { string code = GenericSetup + decl; var analysis = await GetAnalysisAsync(code); - analysis.Diagnostics.Should().HaveCount(0); + analysis.Diagnostics.Should().BeEmpty(); } [DataRow("x = 'str' + 5 #noqa")] @@ -48,7 +51,7 @@ public async Task IgnoreGenerics(string decl) { [DataTestMethod, Priority(0)] public async Task IgnoreBadBinaryOp(string code) { var analysis = await GetAnalysisAsync(code); - analysis.Diagnostics.Should().HaveCount(0); + analysis.Diagnostics.Should().BeEmpty(); } [DataRow("x = 'str' + 5 # noqa")] @@ -56,18 +59,30 @@ public async Task IgnoreBadBinaryOp(string code) { [DataTestMethod, Priority(0)] public async Task IgnoreNoQAWithSpace(string code) { var analysis = await GetAnalysisAsync(code); - analysis.Diagnostics.Should().HaveCount(0); + analysis.Diagnostics.Should().BeEmpty(); } + [TestMethod, Priority(0)] + public async Task VarNamedNoQAStillGivesDiagnostic() { + const string code = @" +noqa = 1 + 'str' +"; + var analysis = await GetAnalysisAsync(code); + analysis.Diagnostics.Should().HaveCount(1); - + var diagnostic = analysis.Diagnostics.ElementAt(0); + diagnostic.ErrorCode.Should().Be(ErrorCodes.UnsupportedOperandType); + diagnostic.Message.Should().Be(Resources.UnsupporedOperandType.FormatInvariant("+", "int", "str")); + diagnostic.SourceSpan.Should().Be(2, 8, 2, 17); + diagnostic.Severity.Should().Be(Severity.Error); + } [DataRow("x = y #noqa")] [DataRow("x = z + 2 #noqa")] [DataTestMethod, Priority(0)] public async Task IgnoreUndefinedVar(string code) { var d = await LintAsync(code); - d.Should().HaveCount(0); + d.Should().BeEmpty(); } [TestMethod, Priority(0)] @@ -76,7 +91,7 @@ public async Task IgnoreMissingImport() { from fake_module import User #noqa "; var analysis = await GetAnalysisAsync(code); - analysis.Diagnostics.Should().HaveCount(0); + analysis.Diagnostics.Should().BeEmpty(); } private async Task> LintAsync(string code, InterpreterConfiguration configuration = null) { From ecd78277308c5990eb998eb07a695873bf024318 Mon Sep 17 00:00:00 2001 From: Cameron Trando Date: Thu, 11 Jul 2019 11:50:17 -0700 Subject: [PATCH 08/14] Shortening up logic --- src/Analysis/Ast/Impl/Extensions/AstExtensions.cs | 10 +--------- .../Ast/Impl/Extensions/PythonModuleExtensions.cs | 4 ++-- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/src/Analysis/Ast/Impl/Extensions/AstExtensions.cs b/src/Analysis/Ast/Impl/Extensions/AstExtensions.cs index 4510a0513..f6649bf01 100644 --- a/src/Analysis/Ast/Impl/Extensions/AstExtensions.cs +++ b/src/Analysis/Ast/Impl/Extensions/AstExtensions.cs @@ -62,15 +62,7 @@ public static bool IsInsideComment(this PythonAst ast, SourceLocation location) } internal static bool HasComment(this PythonAst ast, PythonModule module, string comment, int lineNum) { - var commentLines = ast.CommentLocations.Select(c => c.Line).ToArray(); - var match = Array.BinarySearch(commentLines, lineNum); - - // line is not a comment - if (match < 0 || match >= commentLines.Length) { - return false; - } - - string line = module.GetLine(lineNum); + string line = module.GetLine(ast, lineNum); int commentPos = line.IndexOf('#'); if (commentPos < 0) { diff --git a/src/Analysis/Ast/Impl/Extensions/PythonModuleExtensions.cs b/src/Analysis/Ast/Impl/Extensions/PythonModuleExtensions.cs index c89368680..4769d5b02 100644 --- a/src/Analysis/Ast/Impl/Extensions/PythonModuleExtensions.cs +++ b/src/Analysis/Ast/Impl/Extensions/PythonModuleExtensions.cs @@ -33,10 +33,10 @@ internal static void AddAstNode(this IPythonModule module, object o, Node n) /// Returns the string line corresponding to the given location /// /// The line number - internal static string GetLine(this PythonModule module, int line) { + internal static string GetLine(this PythonModule module, PythonAst ast, int line) { SourceLocation source = new SourceLocation(line, 1); string content = module.Content; - var start = GetAst(module).LocationToIndex(source); + var start = ast.LocationToIndex(source); var end = start; if (string.IsNullOrEmpty(content)) { From a1bef03e5f21fd1a340cdadcdd0467a3b8de56d9 Mon Sep 17 00:00:00 2001 From: Cameron Trando Date: Thu, 11 Jul 2019 11:52:13 -0700 Subject: [PATCH 09/14] getting rid of cast --- src/Analysis/Ast/Impl/Diagnostics/DiagnosticsEntry.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Analysis/Ast/Impl/Diagnostics/DiagnosticsEntry.cs b/src/Analysis/Ast/Impl/Diagnostics/DiagnosticsEntry.cs index 6b14cb53f..c0ff66ba6 100644 --- a/src/Analysis/Ast/Impl/Diagnostics/DiagnosticsEntry.cs +++ b/src/Analysis/Ast/Impl/Diagnostics/DiagnosticsEntry.cs @@ -55,7 +55,7 @@ public DiagnosticsEntry(string message, SourceSpan span, string errorCode, Sever public bool ShouldReport(IPythonModule module) { // module should always be a user written python module - if (module.ModuleType != ModuleType.User || !(module is PythonModule pythonModule)) { + if (module.ModuleType != ModuleType.User) { return false; } From 4c77f6f7fe5359bfac89c47a4045b3af3947b44e Mon Sep 17 00:00:00 2001 From: Cameron Trando Date: Thu, 11 Jul 2019 11:54:13 -0700 Subject: [PATCH 10/14] Adding cast back, is necessary for GetLine method --- src/Analysis/Ast/Impl/Diagnostics/DiagnosticsEntry.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Analysis/Ast/Impl/Diagnostics/DiagnosticsEntry.cs b/src/Analysis/Ast/Impl/Diagnostics/DiagnosticsEntry.cs index c0ff66ba6..6b14cb53f 100644 --- a/src/Analysis/Ast/Impl/Diagnostics/DiagnosticsEntry.cs +++ b/src/Analysis/Ast/Impl/Diagnostics/DiagnosticsEntry.cs @@ -55,7 +55,7 @@ public DiagnosticsEntry(string message, SourceSpan span, string errorCode, Sever public bool ShouldReport(IPythonModule module) { // module should always be a user written python module - if (module.ModuleType != ModuleType.User) { + if (module.ModuleType != ModuleType.User || !(module is PythonModule pythonModule)) { return false; } From 3bc3413553de421c9e7c26de47638352c8346b67 Mon Sep 17 00:00:00 2001 From: Cameron Trando Date: Thu, 11 Jul 2019 15:57:28 -0700 Subject: [PATCH 11/14] Moving to extension approach --- .../Ast/Impl/Diagnostics/DiagnosticsEntry.cs | 4 ++-- src/Analysis/Ast/Impl/Extensions/AstExtensions.cs | 3 ++- .../Ast/Impl/Extensions/PythonModuleExtensions.cs | 13 +++++++------ 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/Analysis/Ast/Impl/Diagnostics/DiagnosticsEntry.cs b/src/Analysis/Ast/Impl/Diagnostics/DiagnosticsEntry.cs index 6b14cb53f..0d7cce581 100644 --- a/src/Analysis/Ast/Impl/Diagnostics/DiagnosticsEntry.cs +++ b/src/Analysis/Ast/Impl/Diagnostics/DiagnosticsEntry.cs @@ -55,11 +55,11 @@ public DiagnosticsEntry(string message, SourceSpan span, string errorCode, Sever public bool ShouldReport(IPythonModule module) { // module should always be a user written python module - if (module.ModuleType != ModuleType.User || !(module is PythonModule pythonModule)) { + if (module.ModuleType != ModuleType.User) { return false; } - if (module.GetAst().HasComment(pythonModule, "noqa", SourceSpan.Start.Line)) { + if (module.GetAst().HasComment(module, "noqa", SourceSpan.Start.Line)) { return false; } diff --git a/src/Analysis/Ast/Impl/Extensions/AstExtensions.cs b/src/Analysis/Ast/Impl/Extensions/AstExtensions.cs index f6649bf01..47516bc49 100644 --- a/src/Analysis/Ast/Impl/Extensions/AstExtensions.cs +++ b/src/Analysis/Ast/Impl/Extensions/AstExtensions.cs @@ -18,6 +18,7 @@ using System.Linq; using Microsoft.Python.Analysis.Analyzer.Expressions; using Microsoft.Python.Analysis.Modules; +using Microsoft.Python.Analysis.Types; using Microsoft.Python.Core.Text; using Microsoft.Python.Parsing; using Microsoft.Python.Parsing.Ast; @@ -61,7 +62,7 @@ public static bool IsInsideComment(this PythonAst ast, SourceLocation location) return ast.CommentLocations[match].Column < location.Column; } - internal static bool HasComment(this PythonAst ast, PythonModule module, string comment, int lineNum) { + internal static bool HasComment(this PythonAst ast, IPythonModule module, string comment, int lineNum) { string line = module.GetLine(ast, lineNum); int commentPos = line.IndexOf('#'); diff --git a/src/Analysis/Ast/Impl/Extensions/PythonModuleExtensions.cs b/src/Analysis/Ast/Impl/Extensions/PythonModuleExtensions.cs index 4769d5b02..6926cec09 100644 --- a/src/Analysis/Ast/Impl/Extensions/PythonModuleExtensions.cs +++ b/src/Analysis/Ast/Impl/Extensions/PythonModuleExtensions.cs @@ -33,17 +33,18 @@ internal static void AddAstNode(this IPythonModule module, object o, Node n) /// Returns the string line corresponding to the given location /// /// The line number - internal static string GetLine(this PythonModule module, PythonAst ast, int line) { + internal static string GetLine(this IPythonModule module, PythonAst ast, int line) { + string content = module.Analysis?.Document?.Content; + if (string.IsNullOrEmpty(content)) { + return string.Empty; + } + SourceLocation source = new SourceLocation(line, 1); - string content = module.Content; var start = ast.LocationToIndex(source); var end = start; - if (string.IsNullOrEmpty(content)) { - return content; - } - for (; end < content.Length && content[end] != '\n' && content[end] != '\r'; end++); + for (; end < content.Length && content[end] != '\n' && content[end] != '\r'; end++) ; return content.Substring(start, end - start); } } From e1e424caa19b8b09c649bc1fd7a7b2af74d9fc49 Mon Sep 17 00:00:00 2001 From: Cameron Trando Date: Fri, 12 Jul 2019 14:45:17 -0700 Subject: [PATCH 12/14] Moving things around --- .../Ast/Impl/Diagnostics/DiagnosticsEntry.cs | 6 ++-- .../Ast/Impl/Extensions/AstExtensions.cs | 11 ------- .../Impl/Extensions/PythonModuleExtensions.cs | 30 ++++++++++++++----- 3 files changed, 26 insertions(+), 21 deletions(-) diff --git a/src/Analysis/Ast/Impl/Diagnostics/DiagnosticsEntry.cs b/src/Analysis/Ast/Impl/Diagnostics/DiagnosticsEntry.cs index 0d7cce581..f9e70f117 100644 --- a/src/Analysis/Ast/Impl/Diagnostics/DiagnosticsEntry.cs +++ b/src/Analysis/Ast/Impl/Diagnostics/DiagnosticsEntry.cs @@ -15,6 +15,7 @@ using Microsoft.Python.Analysis.Modules; using Microsoft.Python.Analysis.Types; +using Microsoft.Python.Core; using Microsoft.Python.Core.Text; using Microsoft.Python.Parsing; @@ -54,12 +55,13 @@ public DiagnosticsEntry(string message, SourceSpan span, string errorCode, Sever public DiagnosticSource Source { get; } public bool ShouldReport(IPythonModule module) { - // module should always be a user written python module + // Only report for user written modules if (module.ModuleType != ModuleType.User) { return false; } - if (module.GetAst().HasComment(module, "noqa", SourceSpan.Start.Line)) { + // If user specifies #noqa, then do not report diagnostic + if (module.GetComment(SourceSpan.Start.Line).EqualsIgnoreCase("noqa")) { return false; } diff --git a/src/Analysis/Ast/Impl/Extensions/AstExtensions.cs b/src/Analysis/Ast/Impl/Extensions/AstExtensions.cs index 47516bc49..65234d56d 100644 --- a/src/Analysis/Ast/Impl/Extensions/AstExtensions.cs +++ b/src/Analysis/Ast/Impl/Extensions/AstExtensions.cs @@ -62,17 +62,6 @@ public static bool IsInsideComment(this PythonAst ast, SourceLocation location) return ast.CommentLocations[match].Column < location.Column; } - internal static bool HasComment(this PythonAst ast, IPythonModule module, string comment, int lineNum) { - string line = module.GetLine(ast, lineNum); - - int commentPos = line.IndexOf('#'); - if (commentPos < 0) { - return false; - } - - return line.Substring(commentPos + 1).Contains(comment); - } - public static bool IsInsideString(this PythonAst ast, SourceLocation location) { var index = ast.LocationToIndex(location); return ast.FindExpression(index, new FindExpressionOptions { Literals = true }) != null; diff --git a/src/Analysis/Ast/Impl/Extensions/PythonModuleExtensions.cs b/src/Analysis/Ast/Impl/Extensions/PythonModuleExtensions.cs index 6926cec09..9a64c1e33 100644 --- a/src/Analysis/Ast/Impl/Extensions/PythonModuleExtensions.cs +++ b/src/Analysis/Ast/Impl/Extensions/PythonModuleExtensions.cs @@ -15,7 +15,6 @@ using Microsoft.Python.Analysis.Modules; using Microsoft.Python.Analysis.Types; -using Microsoft.Python.Core; using Microsoft.Python.Core.Text; using Microsoft.Python.Parsing.Ast; @@ -30,22 +29,37 @@ internal static void AddAstNode(this IPythonModule module, object o, Node n) => ((IAstNodeContainer)module).AddAstNode(o, n); /// - /// Returns the string line corresponding to the given location + /// Returns the string line corresponding to the given line number /// - /// The line number - internal static string GetLine(this IPythonModule module, PythonAst ast, int line) { - string content = module.Analysis?.Document?.Content; + /// The line number + internal static string GetLine(this IPythonModule module, int lineNum) { + string content = module.GetAst().Module; if (string.IsNullOrEmpty(content)) { return string.Empty; } - SourceLocation source = new SourceLocation(line, 1); - var start = ast.LocationToIndex(source); + SourceLocation source = new SourceLocation(lineNum, 1); + var start = module.GetAst().LocationToIndex(source); var end = start; for (; end < content.Length && content[end] != '\n' && content[end] != '\r'; end++) ; - return content.Substring(start, end - start); + return content.Substring(start, end - start).Trim(); + } + + /// + /// Returns the comment corresponding to the given line or null if there is no comment + /// + /// The line number + internal static string GetComment(this IPythonModule module, int lineNum) { + string line = module.GetLine(lineNum); + + int commentPos = line.IndexOf('#'); + if (commentPos < 0) { + return null; + } + + return line.Substring(commentPos + 1); } } } From 483fbd2a5ce323ea12e70013480f58107bda635a Mon Sep 17 00:00:00 2001 From: Cameron Trando Date: Fri, 12 Jul 2019 17:20:02 -0700 Subject: [PATCH 13/14] Fixing issues with having a space or tab in front of #noqa --- .../Impl/Extensions/PythonModuleExtensions.cs | 6 +++--- src/Analysis/Ast/Test/LintNoQATests.cs | 17 +++++++++++++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/Analysis/Ast/Impl/Extensions/PythonModuleExtensions.cs b/src/Analysis/Ast/Impl/Extensions/PythonModuleExtensions.cs index 9a64c1e33..ea9e626db 100644 --- a/src/Analysis/Ast/Impl/Extensions/PythonModuleExtensions.cs +++ b/src/Analysis/Ast/Impl/Extensions/PythonModuleExtensions.cs @@ -33,7 +33,7 @@ internal static void AddAstNode(this IPythonModule module, object o, Node n) /// /// The line number internal static string GetLine(this IPythonModule module, int lineNum) { - string content = module.GetAst().Module; + string content = module.Analysis?.Document?.Content; if (string.IsNullOrEmpty(content)) { return string.Empty; } @@ -44,7 +44,7 @@ internal static string GetLine(this IPythonModule module, int lineNum) { for (; end < content.Length && content[end] != '\n' && content[end] != '\r'; end++) ; - return content.Substring(start, end - start).Trim(); + return content.Substring(start, end - start).Trim('\t', ' '); } /// @@ -59,7 +59,7 @@ internal static string GetComment(this IPythonModule module, int lineNum) { return null; } - return line.Substring(commentPos + 1); + return line.Substring(commentPos + 1).Trim('\t', ' '); } } } diff --git a/src/Analysis/Ast/Test/LintNoQATests.cs b/src/Analysis/Ast/Test/LintNoQATests.cs index def191ebe..9cf42e1b6 100644 --- a/src/Analysis/Ast/Test/LintNoQATests.cs +++ b/src/Analysis/Ast/Test/LintNoQATests.cs @@ -62,6 +62,23 @@ public async Task IgnoreNoQAWithSpace(string code) { analysis.Diagnostics.Should().BeEmpty(); } + [DataRow("x = 'str' + 5 # noqa")] + [DataRow("x = float(1) * 'str' # noqa")] + [DataTestMethod, Priority(0)] + public async Task IgnoreNoQAWithTab(string code) { + var analysis = await GetAnalysisAsync(code); + analysis.Diagnostics.Should().BeEmpty(); + } + + [DataRow("x = 'str' + 5 #NOQA")] + [DataRow("x = float(1) * 'str' # NOQA")] + [DataRow("x = float(1) * 'str' # NOQA")] + [DataTestMethod, Priority(0)] + public async Task IgnoreNoQAUppercase(string code) { + var analysis = await GetAnalysisAsync(code); + analysis.Diagnostics.Should().BeEmpty(); + } + [TestMethod, Priority(0)] public async Task VarNamedNoQAStillGivesDiagnostic() { const string code = @" From b23e59daaec8c1e3703d6e63f8ce4fff3a5878ce Mon Sep 17 00:00:00 2001 From: Cameron Trando Date: Mon, 15 Jul 2019 10:11:42 -0700 Subject: [PATCH 14/14] Returning empty string on no comment --- src/Analysis/Ast/Impl/Extensions/PythonModuleExtensions.cs | 4 ++-- src/Analysis/Ast/Test/LintNoQATests.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Analysis/Ast/Impl/Extensions/PythonModuleExtensions.cs b/src/Analysis/Ast/Impl/Extensions/PythonModuleExtensions.cs index ea9e626db..0cdbcb0f4 100644 --- a/src/Analysis/Ast/Impl/Extensions/PythonModuleExtensions.cs +++ b/src/Analysis/Ast/Impl/Extensions/PythonModuleExtensions.cs @@ -48,7 +48,7 @@ internal static string GetLine(this IPythonModule module, int lineNum) { } /// - /// Returns the comment corresponding to the given line or null if there is no comment + /// Returns the comment corresponding to the given line or an empty string if there is no comment /// /// The line number internal static string GetComment(this IPythonModule module, int lineNum) { @@ -56,7 +56,7 @@ internal static string GetComment(this IPythonModule module, int lineNum) { int commentPos = line.IndexOf('#'); if (commentPos < 0) { - return null; + return string.Empty; } return line.Substring(commentPos + 1).Trim('\t', ' '); diff --git a/src/Analysis/Ast/Test/LintNoQATests.cs b/src/Analysis/Ast/Test/LintNoQATests.cs index 9cf42e1b6..92c1bf924 100644 --- a/src/Analysis/Ast/Test/LintNoQATests.cs +++ b/src/Analysis/Ast/Test/LintNoQATests.cs @@ -5,8 +5,8 @@ using Microsoft.Python.Analysis.Analyzer; using Microsoft.Python.Analysis.Core.Interpreter; using Microsoft.Python.Analysis.Diagnostics; -using Microsoft.Python.Core; using Microsoft.Python.Analysis.Tests.FluentAssertions; +using Microsoft.Python.Core; using Microsoft.Python.Parsing; using Microsoft.Python.Parsing.Tests; using Microsoft.VisualStudio.TestTools.UnitTesting;