search

openrewrite / rewrite-python

OpenRewrite recipes for Python.
Apache License 2.0
12 stars 5 forks source link

Support python stub files #52

Closed JLLeitschuh closed 1 year ago

JLLeitschuh commented 1 year ago

Signed-off-by: Jonathan Leitschuh Jonathan.Leitschuh@gmail.com

JLLeitschuh commented 1 year ago

Currently seeing this error. It looks like arguments are getting re-ordered, which looks like it's likely a problem outside the scope of this change?

org.opentest4j.AssertionFailedError: [When parsing and printing the source code back to text without modifications, the printed source didn't match the original source code. This means there is a bug in the parser implementation itself. Please open an issue to report this, providing a sample of the code that generated this error!] 
expected: 
  "# Variables with annotations do not need to be assigned a value.
  # So by convention, we omit them in the stub file.
  x: int

  # Function bodies cannot be completely removed. By convention,
  # we replace them with `...` instead of the `pass` statement.
  def func_1(code: str) -> int: ...

  # We can do the same with default arguments.
  def func_2(a: int, b: int = ...) -> int: ..."
 but was: 
  "# Variables with annotations do not need to be assigned a value.
  # So by convention, we omit them in the stub file.
  x: int

  # Function bodies cannot be completely removed. By convention,
  # we replace them with `...` instead of the `pass` statement.
  def func_1(code: str) -> int: ...

  # We can do the same with default arguments.
  def func_2(a: int, b = ...: int) -> int: ..."
JLLeitschuh commented 1 year ago

@garyolsen do you have any insights into why the arguments are getting re-ordered, which is causing the tests to fail? I have no insights here.

garyolsen commented 1 year ago

@JLLeitschuh Yes, this is unrelated—the implementation of type hints in our model is being rendered incorrectly when there is a default value for a parameter.

JLLeitschuh commented 1 year ago

Should that get fixed first before this gets merged then? Do you have the cycles to fix that?

garyolsen commented 1 year ago

I think it's fine to merge this first—I won't be able to look at the parsing/printing issue for at least a few days. Just omit the failing test for now and I'll add it back in when I fix the underlying issue.

traceyyoshima commented 1 year ago

Hi @JLLeitschuh! I'm working on adding type attribution to the PythonParser and discovered that the CoreApplicationEnvironment requires the PyiParserDefinition to be configured to work. Issue link.

I'm still sorting out the details about configuring the workspace so that pyi files will resolve correctly, but I expect this PR will result in errors. Can we wait on this until I sort out more details?

JLLeitschuh commented 1 year ago

Sure, better to do it right than fast. What's the ETA?

traceyyoshima commented 1 year ago

Sure, better to do it right than fast. What's the ETA?

Thank you - I agree. I created a PythonCoreApplicationEnvironment, but I am still sorting out how to configure dependencies and IntelliJ's virtual file system to resolve types. PYI files are set into a specific workspace, but I need to figure out how that happens. I need a few days to sort out the requirements and will provide an ETA afterward.

JLLeitschuh commented 1 year ago

If you've got a plan here, I think I'll close this pull request