Ignacio-Freire commented 1 month ago

Problem

When trying to use basic type checking (PyLance language server) with PyNest on custom injectable classes I keep getting the following warning:

Expected type expression but received "str"Pylance[reportGeneralTypeIssues](https://github.com/microsoft/pyright/blob/main/docs/configuration.md#reportGeneralTypeIssues)
(class) ExamplesService

The application still works as expected but this results in autocompletion not being able to work properly. This is becoming an issue when working even in the beginning of an app with already 5 modules.

Tested various approaches but ended up realizing that for some reason the @dataclass decorator is incompatible with the @Injectable one. If I remove the latter (which of course defeats the porpoise of the lib) then PyLance recognizes the class as a type properly.

One extra thing I noticed, which may or may not be related, is an argument error on the @Injectable decoractor

Argument of type "type[ExamplesService]" cannot be assigned to parameter "cls" of type "str" in function "Injectable"
  "type[type]" is incompatible with "type[str]"Pylance[reportArgumentType](https://github.com/microsoft/pyright/blob/main/docs/configuration.md#reportArgumentType)
(function) def Injectable(cls: str = None) -> str

Steps to Reproduce

1. Install Pynest and create app

pip install pynest-api
pynest create-nest-app -n my_app_name
cd my_app_name

2. Create empty environment and activate

python3.11 -m venv venv
source venv/bin/activate

3. Install requirements

pip3.11 install -r requirements.txt

4. Create example as shown on Blank Application Tutorial

pynest g module -n examples

4. Install latest version of PyLance extension for VSCode and add config

or add to settings.json

    "python.analysis.typeCheckingMode": "basic",

Expected

The expected behavior is for the classes being properly recognized as Types as well as @Injectable decorator being properly recognized.

Ignacio-Freire commented 1 month ago

@ItayTheDar Created PR that addresses this issue, let me know what you think

ItayTheDar commented 1 month ago

@Ignacio-Freire Hi!

I just saw your pr now and I'm super excited! I will go through it more deeply to see if there any side effects to this change, but overall seems perfectly fine, it is passes all the unit and integration tests. I must say that i can't say why i chose to assign the cls input to string. This was So thank you for drill down into the codebase and fix things from source. I'm sure that there are many other things to do to optimize pynest and make it follows best practices.

ItayTheDar commented 1 month ago

@Ignacio-Freire I've added 2 comment on the pr

Ignacio-Freire commented 4 weeks ago

@Ignacio-Freire I've added 2 comment on the pr

@ItayTheDar Addressed! Thank you!

ItayTheDar commented 4 weeks ago

@Ignacio-Freire feel free to merge the pr

Ignacio-Freire commented 4 weeks ago

@Ignacio-Freire feel free to merge the pr

@ItayTheDar Cannot merge it myself 😁

"Only those with write access to this repository can merge pull requests."

ItayTheDar commented 3 weeks ago

Ohhh 😧 So i will merge your pr, and this is another chance to thank you for noticed this broken logic and fix it. You are more than welcome to share your ideas and thoughts about PyNest, this is extremely helpful!