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

feat: python language features #991

Open
wants to merge 28 commits into
base: main
Choose a base branch
from

Conversation

jingjiajie
Copy link
Contributor

New libs are installed, please run npm install

Summary
"Pytight" language server is integrated, which is the Microsoft's official python language server.

Thanks to the Pyright, the following features are supported:
Code completion
Hover hint
Outline for python code

and functions shown in the picture:
a

Due to reasons, the following features are not yet included:
Python code formatting: It's not included in the Pyright language server. It has to be a feature we separately implement in the future.
Folding range for Python code: Because we set to use the Language Server side Folding range parsing rather than using the configuration file, it only recognizes the Folding ranges we returned from language server. However, the Python folding range is naturally implemented with configuration file, so it's ignored. We have to treat it as a separate feature and manually implement it with our language server in the future.

a simple test code:

proc python;
submit;
        class Jonas:
            def name(self) -> str:
                return "Jonas"

        def salute() -> None:
            jonas = Jonas()
            print("This man is %s!" % jonas.name())
endsubmit;
run;

proc python;
    submit;
salute()

@jingjiajie
Copy link
Contributor Author

@scnwwu In main branch, formatting the above test code, the code disappearing problem occurs again just like before. Would you mind to have a check again? :)

@Zhirong2022
Copy link
Collaborator

@scnwwu In main branch, formatting the above test code, the code disappearing problem occurs again just like before. Would you mind to have a check again? :)

@jingjiajie, I have raised the issue #992 to track the changes. Thanks.

@scnwwu
Copy link
Member

scnwwu commented May 20, 2024

@jingjiajie, I failed to build your PR code on my PC.
ENOENT: no such file or directory
vscode-sas-extension\server\node_modules\pyright-internal-lsp\dist\packages\pyright-internal\typeshed-fallback
I did npm install but still no such directory

@jingjiajie
Copy link
Contributor Author

@jingjiajie, I failed to build your PR code on my PC. ENOENT: no such file or directory vscode-sas-extension\server\node_modules\pyright-internal-lsp\dist\packages\pyright-internal\typeshed-fallback I did npm install but still no such directory

Fixed

@scnwwu
Copy link
Member

scnwwu commented May 29, 2024

There's a UX problem with this PR. The context menu item "Go to Definition", "Go to Declaration" etc. now always display. For most SAS code except PROC PYTHON, they have no functionality. It may be confusing for some user who rarely use Python.

@scnwwu
Copy link
Member

scnwwu commented May 29, 2024

Got unexpected "p" is not defined error with below code in the editor

proc python terminate;
submit;
#Reference to variable defined in previous PROC PYTHON call
print("x = " + x)
def my_function():
    print("Inside the proc step")
endsubmit;
run;

Got unexpected String literal is unterminated error with below code in the editor

proc python;
  interactive;
  x = '''
    endinteractive
  '''
  print('hello')
endinteractive;
run;

@scnwwu
Copy link
Member

scnwwu commented May 29, 2024

It does NOT work in browser build.

@jingjiajie
Copy link
Contributor Author

There's a UX problem with this PR. The context menu item "Go to Definition", "Go to Declaration" etc. now always display. For most SAS code except PROC PYTHON, they have no functionality. It may be confusing for some user who rarely use Python.

@scnwwu It's controlled by the InitializeResult returned by onInitialize() at server side. I'm not sure if there's a way to dynamically control this. Could have a future investigation. Please let me know if you have more ideas about this design.

It does NOT work in browser build.

I found Microsoft maintained two language servers for Python, one is for desktop(Pyright https://github.com/microsoft/pyright), another is for browser(https://github.com/microsoft/vscode-python). In this PR we integrated the Pyright, so the browser version will become another work after this.
Currently I haven't had a future look at the differecies between them, but it seems that the file system is the biggest difference. Desktop version reads local python executables, libs, etc. while browser version should have totally different mechanisms. Multi-threading is another big difference. There are also many node features required by desktop version for specific features, which browser version implements differently. So that's why there have to be two version.

@scnwwu
Copy link
Member

scnwwu commented Jun 3, 2024

There's a UX problem with this PR. The context menu item "Go to Definition", "Go to Declaration" etc. now always display. For most SAS code except PROC PYTHON, they have no functionality. It may be confusing for some user who rarely use Python.

@scnwwu It's controlled by the InitializeResult returned by onInitialize() at server side. I'm not sure if there's a way to dynamically control this. Could have a future investigation. Please let me know if you have more ideas about this design.

Would you take a look at https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#client_registerCapability
So only register textDocument/definition when found proc python. I don't know if it will work.

@jingjiajie
Copy link
Contributor Author

There's a UX problem with this PR. The context menu item "Go to Definition", "Go to Declaration" etc. now always display. For most SAS code except PROC PYTHON, they have no functionality. It may be confusing for some user who rarely use Python.

@scnwwu It's controlled by the InitializeResult returned by onInitialize() at server side. I'm not sure if there's a way to dynamically control this. Could have a future investigation. Please let me know if you have more ideas about this design.

Would you take a look at https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#client_registerCapability So only register textDocument/definition when found proc python. I don't know if it will work.

Good. It seems to work!

@jingjiajie
Copy link
Contributor Author

Got unexpected "p" is not defined error with below code in the editor

proc python terminate;
submit;
#Reference to variable defined in previous PROC PYTHON call
print("x = " + x)
def my_function():
    print("Inside the proc step")
endsubmit;
run;

Got unexpected String literal is unterminated error with below code in the editor

proc python;
  interactive;
  x = '''
    endinteractive
  '''
  print('hello')
endinteractive;
run;

@scnwwu I found they are caused by Lexer.startFrom(). The CodeZoneManager reads token by calling the Lexer.startFrom() in the middle of embedded code. Without having read "proc python" in previous lines, the Lexer can't regard the token as a part of embedded code, instead it regards it as a regular proc stmt, then gives wrong result.

For example, the correct token should be 'print("x = " + x)' because it's in a proc python block, but if use Lexer.startFrom() from the start position of this line, the token returned is "print", that's wrong.

Is it possible to remove all the occurance of Lexer.startFrom() function, instead use SyntaxProvider.getSyntax()? It returns correct tokens. However it seems to be a big complexity, I'll need your confirmation.

@scnwwu
Copy link
Member

scnwwu commented Jun 13, 2024

Got unexpected "p" is not defined error with below code in the editor

proc python terminate;
submit;
#Reference to variable defined in previous PROC PYTHON call
print("x = " + x)
def my_function():
    print("Inside the proc step")
endsubmit;
run;

Got unexpected String literal is unterminated error with below code in the editor

proc python;
  interactive;
  x = '''
    endinteractive
  '''
  print('hello')
endinteractive;
run;

@scnwwu I found they are caused by Lexer.startFrom(). The CodeZoneManager reads token by calling the Lexer.startFrom() in the middle of embedded code. Without having read "proc python" in previous lines, the Lexer can't regard the token as a part of embedded code, instead it regards it as a regular proc stmt, then gives wrong result.

For example, the correct token should be 'print("x = " + x)' because it's in a proc python block, but if use Lexer.startFrom() from the start position of this line, the token returned is "print", that's wrong.

Is it possible to remove all the occurance of Lexer.startFrom() function, instead use SyntaxProvider.getSyntax()? It returns correct tokens. However it seems to be a big complexity, I'll need your confirmation.

@jingjiajie, please reset context in Lexer.startFrom()
Add this.context.embeddedLangState = EmbeddedLangState.NONE; under
https://github.com/sassoftware/vscode-sas-extension/blob/main/server/src/sas/Lexer.ts#L1027
Then the code zone looks correct. And the 1st snippet looks good.
I don't know what's the problem for the 2nd snippet. Would you provide more details?

@jingjiajie
Copy link
Contributor Author

All reported issues so far have been fixed.

@scnwwu
Copy link
Member

scnwwu commented Jun 19, 2024

@jingjiajie, the build is hang. Would you please address it?

server/package.json Outdated Show resolved Hide resolved
server/package.json Show resolved Hide resolved
client/src/commands/run.ts Show resolved Hide resolved
server/src/python/PyrightLanguageProvider.ts Outdated Show resolved Hide resolved
server/src/server.ts Outdated Show resolved Hide resolved
@Zhirong2022
Copy link
Collaborator

The autocomplete with item 'endsubmit' will be shown if trying to specify a value to a variable.
image

@Zhirong2022
Copy link
Collaborator

It has error validation for a valid code typing.

proc python;

    interactive;
    endinteractive;
run;

image

@tianliang657
Copy link
Collaborator

The autocomplete with item 'endsubmit' will be shown if trying to specify a value to a variable. image

I can met the same issue with the latest extension, but it cannot be reproduced in previous extensions, so it should be a regression issue.

@tianliang657
Copy link
Collaborator

The autocomplete cannot appear until the 'endinteractive' keyword is fully entered when entering 'endinteractive' keyword.
Video

@tianliang657
Copy link
Collaborator

It has error validation for a valid code typing.

proc python;

    interactive;
    endinteractive;
run;

image

If there is a blank line between the “proc python;” and “interactive”, the “interactive” keyword will be thrown error " 'interactive' is not defined", if the blank line does not exist, the error will not appear, this issue cannot be reproduced in previous extensions, so it should be a regression issue.
Video

@Zhirong2022
Copy link
Collaborator

Proper syntax highlighting for the embedded python if using the alias name of 'interactive'
image

@jingjiajie
Copy link
Contributor Author

New reported bugs are all fixed. @Zhirong2022 @tianliang657

@Zhirong2022
Copy link
Collaborator

There is no autocomplete if the first line is a comment. Try to type 'python' or 'print' next to 'proc'

/*  */
proc 

image

@Zhirong2022
Copy link
Collaborator

There is no syntax help for 'endsubmit' and 'endinteractive'.
image

@Zhirong2022
Copy link
Collaborator

Suppose the proc python code written on a line intentionally as below. Delete the code and retype properly, no syntax highlighting for the SAS keyword and no outline update.

proc python;submit;print('test') endsubmit; run;

image

@Zhirong2022
Copy link
Collaborator

The typed python code will be removed in the code below after format code

proc python;
    i;
    print('test')
    endinteractive;
run;

@jingjiajie
Copy link
Contributor Author

There is no autocomplete if the first line is a comment. Try to type 'python' or 'print' next to 'proc'

/*  */
proc 

image

Fixed.

@jingjiajie
Copy link
Contributor Author

There is no syntax help for 'endsubmit' and 'endinteractive'. image

Fixed.

@jingjiajie
Copy link
Contributor Author

Suppose the proc python code written on a line intentionally as below. Delete the code and retype properly, no syntax highlighting for the SAS keyword and no outline update.

proc python;submit;print('test') endsubmit; run;

image

Fixed.

@jingjiajie
Copy link
Contributor Author

The typed python code will be removed in the code below after format code

proc python;
    i;
    print('test')
    endinteractive;
run;

@scnwwu I haven't read through the formatter codes. But just found a logic that is suspicious:
image
It seems that it discards any embedded codes. It could cause this bug.

@scnwwu
Copy link
Member

scnwwu commented Sep 12, 2024

The typed python code will be removed in the code below after format code

proc python;
    i;
    print('test')
    endinteractive;
run;

@scnwwu I haven't read through the formatter codes. But just found a logic that is suspicious: image It seems that it discards any embedded codes. It could cause this bug.

@jingjiajie, the embedded code is handled separately. Would you please try add i beside interactive?
https://github.com/sassoftware/vscode-sas-extension/blob/main/server/src/sas/formatter/parser.ts#L131

j = i - 1;
type = syntax[j].style;

for (let i = 0; i < syntax.length; i++) {
Copy link
Member

Choose a reason for hiding this comment

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

@jingjiajie, would you please provide a detailed explanation for this change? It looks like a fundamental logic. I'd like to verify it carefully. Thank you.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I remember this change is to fix the bug:

There is no autocomplete if the first line is a comment. Try to type 'python' or 'print' next to 'proc'

/*  */
proc 

Because if the starting one token of the whole document is a comment, _token() returned a null value. This change is to fix this bug.

The intention is only to fix that issue, and the remaining logics should keep the same behavior as before.

@Zhirong2022
Copy link
Collaborator

No autocomplete if trying to type proc python or some others under comment line.

data _null_;
    set sashelp.cars;
run;

/*  */

image

@Zhirong2022
Copy link
Collaborator

Error 'Request textDocument/foldingRange failed.' if trying to delete 'endsubmit' at line 6.

proc python terminate;
submit;
#Reference to variable defined in previous PROC PYTHON call
x = "Updating the value for x"
print("x = " + x)
endsubmit;

#Redefining a function called my_function
# def my_function():
#     print("Inside the proc step")
run;

proc python;
submit;
print("x = " + x)
endsubmit;
run;

@Zhirong2022
Copy link
Collaborator

Some code will be always automatically inserted in the first python code block. Type run to load the autocomplete, select 'runtime' from the list.

proc python;
    submit;
    print('test')
    endsubmit;
run;

proc python;
    interactive;

image

@jingjiajie
Copy link
Contributor Author

The typed python code will be removed in the code below after format code

proc python;
    i;
    print('test')
    endinteractive;
run;

@scnwwu I haven't read through the formatter codes. But just found a logic that is suspicious: image It seems that it discards any embedded codes. It could cause this bug.

@jingjiajie, the embedded code is handled separately. Would you please try add i beside interactive? https://github.com/sassoftware/vscode-sas-extension/blob/main/server/src/sas/formatter/parser.ts#L131

@scnwwu Sorry for missed message, I just see your reply. I tried replace i with interactive, then it worked. I found the root cause in the code, and added support for "i" keyword in addition to interactive; Thanks.

@jingjiajie
Copy link
Contributor Author

No autocomplete if trying to type proc python or some others under comment line.

data _null_;
    set sashelp.cars;
run;

/*  */

image

Fixed

@jingjiajie
Copy link
Contributor Author

Error 'Request textDocument/foldingRange failed.' if trying to delete 'endsubmit' at line 6.

proc python terminate;
submit;
#Reference to variable defined in previous PROC PYTHON call
x = "Updating the value for x"
print("x = " + x)
endsubmit;

#Redefining a function called my_function
# def my_function():
#     print("Inside the proc step")
run;

proc python;
submit;
print("x = " + x)
endsubmit;
run;

Fixed.

@jingjiajie
Copy link
Contributor Author

jingjiajie commented Sep 27, 2024

Some code will be always automatically inserted in the first python code block. Type run to load the autocomplete, select 'runtime' from the list.

proc python;
    submit;
    print('test')
    endsubmit;
run;

proc python;
    interactive;

image

@Zhirong2022
This problem is hard to fix because this behavior provided by Pyright is not configurable for us.
Pyright's rule is:
It always automatically insert import clause at the first non-empty and non-comment python code line.

In this case, the first python code line is print('test'), but actually we will insert a hidden if True: line before this line, that is, the same line of submit; clause. That's why you see it insert the import clause at the line of submit clause.

Workaround 1:
You can insert a empty line before print('test')

Workaround 2:
Another workaround is, remove indentation in the first proc python block, like this:

proc python;
submit;
print('test'); 
endsubmit;
run;

So that we'll not insert the hidden if True: line. The print('test') will be the first line.

@Zhirong2022
Copy link
Collaborator

Suppose define a variable named 'endsubmit' or 'endinteractive', it will show SAS syntax help for the variable.
Type 'end' at the second python code block.

proc python;
submit;
endsubmit = 'hello'
endinteractive ='hi'
print('test')
endsubmit;
run;

proc python;
interactive;

endinteractive;
run;

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing Test the pull requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants