Skip to content
This repository was archived by the owner on Apr 14, 2022. It is now read-only.

Added an option to propagate parameter types to base methods #283

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/Analysis/Engine/Impl/AnalysisLimits.cs
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,12 @@ public AnalysisLimits() {
/// </summary>
public bool ProcessCustomDecorators { get; set; }

/// <summary>
/// <c>True</c> to propagate parameter types to base methods.
/// Parameter types always propagate to derived methods.
/// </summary>
public bool PropagateParameterTypeToBaseMethods { get; set; }

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there case when we don't want this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In A: B, B: C case, when we see b:B = ..., b.foo(SomeType()), b can be either of A and B. So, assuming substitution principle, B.foo should propagate to A.foo. However, there's no principle, that would tell, that C.foo should also take an instance of SomeType. At best, its an lucky guess.

So I think its good to have an option.

Now for my purposes, I need it true. But you guys can decide for the default value.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, it is not something that user sets so it it typically set by the application one way or another. #261 may be related.


/// <summary>
/// True to read information from type stub packages.
/// </summary>
Expand Down
2 changes: 1 addition & 1 deletion src/Analysis/Engine/Impl/Analyzer/DDG.cs
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,7 @@ private bool TryImportModule(string modName, bool forceAbsolute, out ModuleRefer
return false;
}

internal List<AnalysisValue> LookupBaseMethods(string name, IEnumerable<IAnalysisSet> mro, Node node, AnalysisUnit unit) {
internal static List<AnalysisValue> LookupBaseMethods(string name, IEnumerable<IAnalysisSet> mro, Node node, AnalysisUnit unit) {
var result = new List<AnalysisValue>();
foreach (var @class in mro.Skip(1)) {
foreach (var curType in @class) {
Expand Down
8 changes: 8 additions & 0 deletions src/Analysis/Engine/Impl/Values/FunctionInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,14 @@ public override IAnalysisSet Call(Node node, AnalysisUnit unit, IAnalysisSet[] a
}
}

if (_analysisUnit.State.Limits.PropagateParameterTypeToBaseMethods
&& _analysisUnit.Scope.OuterScope is ClassScope parentClass) {
var baseMethods = DDG.LookupBaseMethods(Name, parentClass.Class.Mro, AnalysisUnit.Ast, AnalysisUnit);
foreach (FunctionInfo baseMethod in baseMethods.OfType<FunctionInfo>()) {
baseMethod.DoCall(node, unit, baseMethod._analysisUnit, callArgs);
}
}

if (_callsWithClosure != null) {
var chain = new CallChain(node, unit, _callDepthLimit);
var aggregate = GetAggregate(unit);
Expand Down
28 changes: 28 additions & 0 deletions src/Analysis/Engine/Test/InheritanceTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -83,5 +83,33 @@ def foo(self, x):
analysis.Should().HaveClass("Derived").WithFunction("foo")
.WithParameter("x").OfType(BuiltinTypeId.Int);
}

[TestMethod]
public async Task ParameterTypesPropagateToBaseFunctions() {
var code = @"
class Baze:
def foo(self, x):
pass

class Derived(Baze):
def foo(self, x):
pass

Derived().foo(42)
";

using (var server = await new Server().InitializeAsync(PythonVersions.Required_Python36X)) {
server.Analyzer.Limits.PropagateParameterTypeToBaseMethods = true;

var analysis = await server.OpenDefaultDocumentAndGetAnalysisAsync(code);

// the class, for which we know parameter type initially
analysis.Should().HaveClass("Derived").WithFunction("foo")
.WithParameter("x").OfType(BuiltinTypeId.Int);
// its base class
analysis.Should().HaveClass("Baze").WithFunction("foo")
.WithParameter("x").OfType(BuiltinTypeId.Int);
}
}
}
}