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

New rule: fast-api-unused-path-parameter #12632

Closed
Matthieu-LAURENT39 opened this issue Aug 2, 2024 · 3 comments
Closed

New rule: fast-api-unused-path-parameter #12632

Matthieu-LAURENT39 opened this issue Aug 2, 2024 · 3 comments
Labels
rule Implementing or modifying a lint rule

Comments

@Matthieu-LAURENT39
Copy link
Contributor

Matthieu-LAURENT39 commented Aug 2, 2024

Keywords: fastapi, path parameters

Hello, i've been using ruff for a while and i'm really loving it.
I saw that recently some FastAPI rules have been added, and i would like to suggest a new one, fast-api-unused-path-parameter.
It's pretty simple, it verifies that when defining a route that takes in path parameters, the function has matching arguments.

For example, the following code would trigger the rule, as we define a path parameter named "thing_id", but the function doesn't have an argument named "thing_id":

from fastapi import FastAPI

app = FastAPI()

@app.get("/things/{thing_id}")
async def read_thing(id: int, query: str = None):
    return {"thing_id": id, "query": query}

Also of note is that it should ignore any characters after a : in the path parameter name, so this wouldn't trigger the rule:

...

@app.get("/place/{my_path:path}")
def read_path(my_path):
    ...

If this rule is a good fit for ruff, i'd love to make a PR to implement it!

@Matthieu-LAURENT39 Matthieu-LAURENT39 changed the title New rule: fast-api-unused-path-parameters New rule: fast-api-unused-path-parameter Aug 2, 2024
@charliermarsh charliermarsh added the rule Implementing or modifying a lint rule label Aug 3, 2024
charliermarsh pushed a commit that referenced this issue Aug 16, 2024
…12638)

This adds the `fast-api-unused-path-parameter` lint rule, as described
in #12632.

I'm still pretty new to rust, so the code can probably be improved, feel
free to tell me if there's any changes i should make.

Also, i needed to add the `add_parameter` edit function, not sure if it
was in the scope of the PR or if i should've made another one.
@mattmess1221
Copy link

How should routes with path parameters consumed in dependencies behave? Example:

def foo(bar: Annotated[str, Path()]):
  print(bar)


@app.get("/foo/{bar}", dependencies=[Depends(foo)])
async def do_foo():
    pass

@MichaReiser
Copy link
Member

Thanks for proposing a new rule.

Can you help me understand what's still missing from #12638 that shipped as preview rule in 0.6.1?

@AlexWaygood
Copy link
Member

Implemented in #12638

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Development

No branches or pull requests

5 participants