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

raw pathlib.Path truthyness checking #11048

Open
hairmare opened this issue Apr 19, 2024 · 4 comments
Open

raw pathlib.Path truthyness checking #11048

hairmare opened this issue Apr 19, 2024 · 4 comments
Labels
rule Implementing or modifying a lint rule

Comments

@hairmare
Copy link

As a strong proponent of ALL my colleague and I have recently stumbled over PTH (flake8-use-pathlib)

Specifically in an effort to replace os.path we ended up doing this by accident

from pathlib import Path
if Path("does-not-exists"):
    ...

We forgot to Path(...).exists(), it should have been this:

from pathlib import Path
if Path("does-not-exists").exists():
    ...

Given this happened in the context of an if clause. Is ensuring that noone tries to check a bare Path for truthiness a check that could/should be introduced?

@charliermarsh charliermarsh added the rule Implementing or modifying a lint rule label Apr 19, 2024
@charliermarsh
Copy link
Member

That seems like a reasonable rule to me, though not sure how best to categorize it.

@MichaReiser
Copy link
Member

I'm not sure if this rule is too specific. What about other types that are used to check for truthiness?

Should we make the rule more generic so that it warns about all types that don't implement __bool__ and aren't well known types (e.g. list, primitives etc) and are used in a thruthiness check?

@trim21
Copy link

trim21 commented Apr 22, 2024

I'm not sure if this rule is too specific. What about other types that are used to check for truthiness?

Should we make the rule more generic so that it warns about all types that don't implement __bool__ and aren't well known types (e.g. list, primitives etc) and are used in a thruthiness check?

I have another case for this...

import xml.etree.ElementTree

et: xml.etree.ElementTree.Element = xml.etree.ElementTree.fromstring(
    """
<rss version="2.0">
<channel>
<title>title</title>
<link>
<![CDATA[ https://example.com ]]>
</link>
<pubDate>Mon, 22 Apr 2024 16:50:40 +0800</pubDate>
<item>
<title>
<![CDATA[ a title ]]>
</title>
<link>https://exmaple.com/not-expected-url</link>
<enclosure url="https://example.com/expected-url" length="16306828171" type="application/x-bittorrent"/>
<guid isPermaLink="false">guid</guid>
</item>
</channel>
</rss>
"""
)

raw = []

for item in et.findall("channel/item"):
    enclosure = item.find("./enclosure")
    # gotcha !
    # need to use `enclosure is not None`
    if enclosure:
        url = enclosure.attrib.get("url")
    else:
        # torrentleech
        url = item.findtext("./link")

    print(url)

@hairmare
Copy link
Author

A general __bool__ check does sound like an interesting approach worth further exploring and would help uncovering the error we initially encountered.

The etree example wouldn't be covered by a rule generalized in this way given the Element returned from a find lookup does implement __bool__. In most XML use cases, i usually recommend ensuring the data can be trusted by doing previous validation using XML tooling like XML Schema Definitions.

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

4 participants