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

NullReferenceException getting URI absolute path in PythonModule.Definition #1018

Description

   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.

Activity

  1. AlexanderSher commented on Apr 26, 2019

    @AlexanderSher
    Contributor

    Uri can be null only for SentinelModule.

  2. AlexanderSher commented on Apr 26, 2019

    @AlexanderSher
    Contributor

    Do you have repro steps?

  3. jakebailey commented on Apr 26, 2019

    @jakebailey
    MemberAuthor

    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.

  4. AlexanderSher commented on Apr 26, 2019

    @AlexanderSher
    Contributor

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

  5. MikhailArkhipov commented on May 1, 2019

    @MikhailArkhipov

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

  6. AlexanderSher commented on May 1, 2019

    @AlexanderSher
    Contributor

    How is it possible to get Sentinel in FindDefinition?

  7. MikhailArkhipov commented on May 1, 2019

    @MikhailArkhipov

    Unresolved import?

  8. AlexanderSher commented on May 1, 2019

    @AlexanderSher
    Contributor

    FindDefinition checks analysis?.Ast at the very beginning

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

    So it won't proceed for unresolved import.

  9. MikhailArkhipov commented on May 1, 2019

    @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;
            }
  10. AlexanderSher commented on May 1, 2019

    @AlexanderSher
    Contributor

    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?

  11. MikhailArkhipov commented on May 1, 2019

    @MikhailArkhipov

    Technically yes

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

    such as we didn't find typing

  12. MikhailArkhipov commented on May 1, 2019

    @MikhailArkhipov

    And in PythonVariableModule

    public Uri Uri => Module?.Uri;
  13. AlexanderSher commented on May 1, 2019

    @AlexanderSher
    Contributor

    Then we should probably fail in this place: if module path isn't found, there is no reason to specialize it.

  14. MikhailArkhipov commented on May 1, 2019

    @MikhailArkhipov

    But this

            public PythonVariableModule(string name, IPythonInterpreter interpreter)
                : base(PythonMemberType.Module) {
                Name = name;
                Interpreter = interpreter;
                SetDeclaringModule(this);
            }

    called from

            private PythonVariableModule GetOrCreateVariableModule(in string fullName, in PythonVariableModule parentModule, in string memberName) {
                if (_variableModules.TryGetValue(fullName, out var variableModule)) {
                    return variableModule;
                }
    
                variableModule = new PythonVariableModule(fullName, Interpreter);

    creates module with no Module and hence no URI

  15. AlexanderSher commented on May 1, 2019

    @AlexanderSher
    Contributor

    PythonVariableModule without Module means that it is actually a package. So we shouldn't get into searching for module members in first place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions