Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Documentation symbols not added to exported types in declaration files if they are referenced in a declared global #725

Closed
JohnnyMorganz opened this issue Oct 26, 2022 · 2 comments · Fixed by #727
Assignees
Labels
bug Something isn't working

Comments

@JohnnyMorganz
Copy link
Contributor

For a definition file like so:

declare class Foo
    function bar(self, x: string): number
    function bar(self, x: number): string
end

a documentationSymbol is correctly attached to Foo as @test/globaltype/Foo

However, if I then make the file like the following

declare class Foo
    function bar(self, x: string): number
    function bar(self, x: number): string
end
declare function newFoo(): Foo

Foo no longer gets any documentation symbols attached to itself or its props

Taking a deeper look, it seems to be related to the code at https://github.com/Roblox/luau/blob/54324867df2d91bc527f6de9217b05c3a9ae235f/Analysis/src/Frontend.cpp#L163-L181 and generateDocumentationSymbols
The generate doc symbols function doesn't add symbols if the type is persistent

It seems that, in the first example, the cloned ty is not persistent and so doc symbols get generated for it, but because i've added the declare function newFoo() in the second example, the cloned ty is already persistent, so it fails to add a doc symbol

@JohnnyMorganz JohnnyMorganz added the bug Something isn't working label Oct 26, 2022
@vegorov-rbx
Copy link
Collaborator

Looks like a pass to make types persistent should run separately after everything is cloned and annotated.

@vegorov-rbx
Copy link
Collaborator

Should get fixed later this week.

vegorov-rbx added a commit that referenced this issue Oct 28, 2022
* #719
* Improved `Failed to unify type packs` error message to be reported as
`Type pack 'X' could not be converted into 'Y'`
* #722
* 1% reduction in executed instruction count by removing a check in fast
call dispatch
* Additional fixes to reported error location of OOM errors in VM
* Improve `math.sqrt`, `math.floor` and `math.ceil` performance on
additional compilers and platforms (1-2% geomean improvement including
8-9% on math-cordic)
* All thrown exceptions by Luau analysis are derived from
`Luau::InternalCompilerError`
* When a call site has fewer arguments than required, error now reports
the location of the function name instead of the argument to the
function
* #724
* Fixed #725

Co-authored-by: Arseny Kapoulkine <arseny.kapoulkine@gmail.com>
Co-authored-by: Andy Friesen <afriesen@roblox.com>
RomanKhafizianov pushed a commit to RomanKhafizianov/luau that referenced this issue Nov 27, 2023
* luau-lang/luau#719
* Improved `Failed to unify type packs` error message to be reported as
`Type pack 'X' could not be converted into 'Y'`
* luau-lang/luau#722
* 1% reduction in executed instruction count by removing a check in fast
call dispatch
* Additional fixes to reported error location of OOM errors in VM
* Improve `math.sqrt`, `math.floor` and `math.ceil` performance on
additional compilers and platforms (1-2% geomean improvement including
8-9% on math-cordic)
* All thrown exceptions by Luau analysis are derived from
`Luau::InternalCompilerError`
* When a call site has fewer arguments than required, error now reports
the location of the function name instead of the argument to the
function
* luau-lang/luau#724
* Fixed luau-lang/luau#725

Co-authored-by: Arseny Kapoulkine <arseny.kapoulkine@gmail.com>
Co-authored-by: Andy Friesen <afriesen@roblox.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging a pull request may close this issue.

3 participants