NullReferenceException getting URI absolute path in PythonModule.Definition

[jakebailey]

   at Microsoft.Python.Core.UriExtensions.ToAbsolutePath(Uri uri)
   at Microsoft.Python.Analysis.Modules.PythonModule.get_Definition()
   at Microsoft.Python.LanguageServer.Sources.DefinitionSource.FromMember(IMember m)
   at Microsoft.Python.LanguageServer.Sources.DefinitionSource.FindDefinition(IDocumentAnalysis analysis, SourceLocation location, ILocatedMember& member)
   at Microsoft.Python.LanguageServer.Implementation.Server.GotoDefinition(TextDocumentPositionParams params, CancellationToken cancellationToken)
   at Microsoft.Python.LanguageServer.Implementation.LanguageServer.GotoDefinition(JToken token, CancellationToken cancellationToken)

   at Microsoft.Python.Core.UriExtensions.ToAbsolutePath(Uri uri)
   at Microsoft.Python.Analysis.Modules.PythonModule.get_Definition()
   at Microsoft.Python.LanguageServer.Sources.DefinitionSource.FromMember(IMember m)
   at Microsoft.Python.LanguageServer.Sources.DefinitionSource.FindDefinition(IDocumentAnalysis analysis, SourceLocation location, ILocatedMember& member)
   at Microsoft.Python.LanguageServer.Sources.ReferenceSource.FindAllReferencesAsync(Uri uri, SourceLocation location, ReferenceSearchOptions options, CancellationToken cancellationToken)
   at Microsoft.Python.LanguageServer.Implementation.LanguageServer.FindReferences(JToken token, CancellationToken cancellationToken)

Fix is likely one of:

• Just do Uri?.ToAbsolutePath(); LocationInfo.FilePath is only ever used for comparison and is allowed to be null.
• Pass in PythonModule.FilePath instead of converting Uri.

[AlexanderSher]

Uri can be null only for SentinelModule.

[AlexanderSher]

Do you have repro steps?

[jakebailey]

For PythonModule, you mean? I wonder where the null is coming from, then. Even for untitled files, there will be a URI. Without line information I've been assuming that the URI is null, but it might be a member (Uri.LocalPath?).

I have no repro for this, unfortunately; these are pulled from telemetry.

[AlexanderSher]

LocalPath doesn't return null. If Uri is null for any other type than SentinelModule, then we have a bug in some other place.

[MikhailArkhipov]

For Sentinel the Module?.Analysis.Ast would be null since it has empty analysis. Anyway, I'll add extra checks anyway.

[AlexanderSher]

How is it possible to get Sentinel in FindDefinition?

[MikhailArkhipov]

Unresolved import?

[AlexanderSher]

FindDefinition checks analysis?.Ast at the very beginning

            if (analysis?.Ast == null) {
                return null;
            }

So it won't proceed for unresolved import.

[MikhailArkhipov]

Bunch of code checks FilePath for nulls so I guess there could be cases. Also, SpecializedModule. Ex

        public IPythonModule SpecializeModule(string name, Func<string, IPythonModule> specializationConstructor) {
            var import = CurrentPathResolver.GetModuleImportFromModuleName(name);
            var module = specializationConstructor(import?.ModulePath);
            _specialized[name] = module;
            return module;
        }

[AlexanderSher]

FilePath can be null, Uri can't.
SpecializedModule is abstract, its only derived class is TypingModule. So that would require TypingModule to be created with null or invalid path. Is it possible?

[MikhailArkhipov]

Technically yes

            var import = CurrentPathResolver.GetModuleImportFromModuleName(name);
            var module = specializationConstructor(import?.ModulePath);

such as we didn't find typing