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

Checker for unspecified-encoding is unable to infer attributes from dataclasses #5321

Open
DanielNoord opened this issue Nov 16, 2021 · 5 comments
Labels
dataclasses False Negative 🦋 No message is emitted but something is wrong with the code Help wanted 🙏 Outside help would be appreciated, good for new contributors Needs astroid Brain 🧠 Needs a brain tip in astroid (then an astroid upgrade) Needs astroid update Needs an astroid update (probably a release too) before being mergable Needs PR This issue is accepted, sufficiently specified and now needs an implementation

Comments

@DanielNoord
Copy link
Collaborator

DanielNoord commented Nov 16, 2021

Bug description

A small part of this issue has been fixed in #5332, but the following tests should still be added to tests/functional/u/unspecified_encoding_py38.py.

import dataclasses
from typing import Optional

FILENAME = "test_two.py"


@dataclasses.dataclass
class IOArgs:
    """Dataclass storing information about how to open a file"""

    encoding: Optional[str]
    mode: str


args_good_one = IOArgs(encoding=None, mode="wb")
args_good_two = IOArgs(encoding="utf8", mode="w")
args_bad_one = IOArgs(encoding=None, mode="w")
LOCALE_ENCODING = None
args_bad_two = IOArgs(encoding=LOCALE_ENCODING, mode="w")

file = open(FILENAME, args_good_one.mode, encoding=args_good_one.encoding)
file = open(FILENAME, args_good_two.mode, encoding=args_good_two.encoding)
file = open(FILENAME, args_bad_one.mode, encoding=args_bad_one.encoding)
file = open(FILENAME, args_bad_two.mode, encoding=args_bad_two.encoding)

Configuration

No response

Command used

pylint test.py

Pylint output

No response

Expected behavior

It has something to do with the inference of dataclasses but I couldn't figure it out easily. I only know that astroid handles this in a special way but I have no knowledge of that part of the codebase.
Any help would be appreciated!

Found while working on #5173

Pylint version

Latest commit on `main`

OS / Environment

No response

Additional dependencies

No response

@DanielNoord DanielNoord added Bug 🪲 Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling Crash 💥 A bug that makes pylint crash Help wanted 🙏 Outside help would be appreciated, good for new contributors and removed Bug 🪲 Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Nov 16, 2021
@DanielNoord DanielNoord added this to the 2.12.0 milestone Nov 16, 2021
@Pierre-Sassoulas Pierre-Sassoulas added the Needs astroid update Needs an astroid update (probably a release too) before being mergable label Nov 16, 2021
@Pierre-Sassoulas
Copy link
Member

I don't know what this Proxy class is doing, git log --pretty=short -u -L 128,136:astroid/bases.py does not bring a lot of information either. It seems like it's making everything more complicated though, shouldn't we try to remove the class and handle it in each class with proper typing ? Any idea about that @hippo91 ?

@Pierre-Sassoulas
Copy link
Member

I think this is a duplicate of #5266

@cdce8p
Copy link
Member

cdce8p commented Nov 17, 2021

This seems like a two part issue:

  1. args_good_one.encoding is inferred as <Instance of builtins.str>. It should have been a Const (string) node with value wb. Likely an issue in the dataclass brain.
  2. _check_open_encoded does an unprotected attribute access to mode_arg.value. A fix could be to add an isinstance(mode_arg, nodes.Const) check. I would think this should resolve 'ClassDef' object has no attribute 'value' #5266 as well.

https://github.com/PyCQA/pylint/blob/7d2b6c9bd807dda7502e739c7b9fb2182d89f457/pylint/checkers/stdlib.py#L629

@cdce8p
Copy link
Member

cdce8p commented Nov 20, 2021

I just removed the milestone here. The crash was fixed with #5332 (closed #5266 for it). Should we leave this open for the dataclass inference issue?

@DanielNoord
Copy link
Collaborator Author

I think so! I'll update the OP and title!

@DanielNoord DanielNoord added Needs astroid Brain 🧠 Needs a brain tip in astroid (then an astroid upgrade) False Negative 🦋 No message is emitted but something is wrong with the code and removed Crash 💥 A bug that makes pylint crash labels Nov 21, 2021
@DanielNoord DanielNoord changed the title Crash on dataclasses attributes within open() calls Checker for unspecified-encoding is unable to infer attributes from dataclasses Nov 21, 2021
@Pierre-Sassoulas Pierre-Sassoulas added the Needs PR This issue is accepted, sufficiently specified and now needs an implementation label Jun 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dataclasses False Negative 🦋 No message is emitted but something is wrong with the code Help wanted 🙏 Outside help would be appreciated, good for new contributors Needs astroid Brain 🧠 Needs a brain tip in astroid (then an astroid upgrade) Needs astroid update Needs an astroid update (probably a release too) before being mergable Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
Development

No branches or pull requests

3 participants