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

3.3.x regression treating fstring as a constant: C0103 in scripts #9963

Closed
ColinHarrington opened this issue Sep 24, 2024 · 7 comments
Closed
Labels
Invalid Not a bug, already exists or already fixed

Comments

@ColinHarrington
Copy link

ColinHarrington commented Sep 24, 2024

Bug description

It appears that fstrings in the script scope (not in a function anyway) are being identified as constants and linted inconsistently.

These are new findings in our codebases in the 3.3.x line

#!/usr/bin/env python3
"""
    Example of an what looks to be a pylint regression
"""

def greet(name):
    """CS101"""
    greeting = f"Hello {name}"
    print(greeting)

for thing in ["foo", "bar", "baz"]:
    full_name = f"prefix-{thing}"

greet("world")

Configuration

No response

Command used

pylint example

I also ran some combinations in docker to reproduce the issue:

 docker run -it --rm -v $(pwd)/example:/example python:3.10-alpine /bin/ash -x -c "pip install -q pylint==3.3.1 && pylint --version && pylint /example"

And then I tested multiple versions of pylint with this script:

#!/bin/bash -e

PYTHON_VERSION=3.10
for pylint_version in 3.0.* 3.1.* 3.2.* 3.3.0 3.3.1;
do
    docker run \
        --rm \
        -v "$(pwd)/example":/example \
        python:${PYTHON_VERSION}-alpine \
        /bin/ash +x \
        -c "pip install -q pylint==${pylint_version} 2>1 \
        && pylint --version \
        && pylint /example \
        ; echo \"\$(pylint --version | head -1): \$( [ \$? == 0 ] && echo \"\" || echo \"\")
        \""
done

I also has similar results with Several versions of python (3.9, 3.10, 3.11, 3.12, etc)

Pylint output

************* Module example
/example:12:4: C0103: Constant name "full_name" doesn't conform to UPPER_CASE naming style (invalid-name)

I also tested versions 3.0, 3.1, 3.2 3.3.0 and 3.3.1 with this result:

pylint 3.0.4
astroid 3.0.3
Python 3.10.15 (main, Sep 12 2024, 21:11:58) [GCC 13.2.1 20240309]

------------------------------------
Your code has been rated at 10.00/10

pylint 3.0.4: ✅ 
        
pylint 3.1.1
astroid 3.1.0
Python 3.10.15 (main, Sep 12 2024, 21:11:58) [GCC 13.2.1 20240309]

------------------------------------
Your code has been rated at 10.00/10

pylint 3.1.1: ✅ 
        
pylint 3.2.7
astroid 3.2.4
Python 3.10.15 (main, Sep 12 2024, 21:11:58) [GCC 13.2.1 20240309]

------------------------------------
Your code has been rated at 10.00/10

pylint 3.2.7: ✅ 
        
pylint 3.3.0
astroid 3.3.4
Python 3.10.15 (main, Sep 12 2024, 21:11:58) [GCC 13.2.1 20240309]
************* Module example
/example:12:4: C0103: Constant name "full_name" doesn't conform to UPPER_CASE naming style (invalid-name)

-----------------------------------
Your code has been rated at 8.33/10

pylint 3.3.0: ❌ 
        
pylint 3.3.1
astroid 3.3.4
Python 3.10.15 (main, Sep 12 2024, 21:11:58) [GCC 13.2.1 20240309]
************* Module example
/example:12:4: C0103: Constant name "full_name" doesn't conform to UPPER_CASE naming style (invalid-name)

-----------------------------------
Your code has been rated at 8.33/10

pylint 3.3.1: ❌ 
        

Expected behavior

I expect that an fstring in the script scope would not result in a pylint error C0103 congruent with the behavior prior to 3.3.0

Pylint version

pylint 3.3.1
astroid 3.3.4
Python 3.9.10 (main, May 31 2022, 12:13:35) 
[Clang 13.1.6 (clang-1316.0.21.2.5)]

Also from docker python alpine image:

pylint 3.3.1
astroid 3.3.4
Python 3.10.15 (main, Sep 12 2024, 21:11:58) [GCC 13.2.1 20240309]

OS / Environment

Darwin(intel x64), reproducible in Docker

Additional dependencies

No response

Tasks

Preview Give feedback
No tasks being tracked yet.
@ColinHarrington ColinHarrington added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Sep 24, 2024
@jacobtylerwalls
Copy link
Member

Thanks for the report, but this was a false negative fixed. Now the f-string behaves the same way as regular strings. They each need to conform to your configured naming style.

@jacobtylerwalls jacobtylerwalls closed this as not planned Won't fix, can't repro, duplicate, stale Sep 25, 2024
@jacobtylerwalls jacobtylerwalls added Invalid Not a bug, already exists or already fixed and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Sep 25, 2024
@ColinHarrington
Copy link
Author

ColinHarrington commented Sep 25, 2024

Thanks for the report, but this was a false negative fixed. Now the f-string behaves the same way as regular strings. They each need to conform to your configured naming style.

Then why are the f-strings linted differently within a function vs in the script/global scope in for-loop? Please look at the example for reference.

Am I missing some new configuration here or how do we explain the change in linting behavior?

@jacobtylerwalls
Copy link
Member

There are different naming styles for functions versus global/script scope. This has always been the case for strings, f-strings are consistent with that now. The naming style config is explained in detail here.

You may find further behavior differences for type-annotated module constants, but we're addressing that in a future version: #9771.

@thasti
Copy link

thasti commented Jan 13, 2025

@jacobtylerwalls Could you comment on why f-strings are considered a constant in either of these contexts? This seems very foreign to me (based on PEP 498, which explicitly says 'an f-string is really an expression evaluated at run time, not a constant value').

Consider the code

"""pylint test case"""

for x in range(5):
    fstring = f"{x}"
    print(fstring)

Which throws the same C0103 as OP has shown (considering fstring as a constant). If it were, the following code should be valid:

"""pylint test case"""

fstring = f"{x}"  # does not parse because x is not defined
for x in range(5):
    print(fstring)

The evaluation/substitution of the f-string happens during its assignment, not during the print statement. Renaming the variable to FSTRING would give the impression that its content is the same in every loop iteration, which is not actually the case.

@jacobtylerwalls
Copy link
Member

I hear you, but I was just saying that it's no different than this:

% cat a.py
total = 0
for i in range(5):
    total += 1
% pylint a.py
************* Module a
a.py:1:0: C0114: Missing module docstring (missing-module-docstring)
a.py:1:0: C0103: Constant name "total" doesn't conform to UPPER_CASE naming style (invalid-name)

------------------------------------------------------------------
Your code has been rated at 3.33/10 (previous run: 0.00/10, +3.33)

Without diving into the git blame, I assume that the authors of this check decided that everything defined at the module level should be treated like a constant for the purposes of invalid-name. They probably wanted to discourage having module-level variables reassigned. Is that mixing two checks together? Probably. We could discuss that in a separate issue, but it's not really related to f-strings.

@jacobtylerwalls
Copy link
Member

Your example with print doesn't involve a reassignment, I know, so I take your point that caps would be misleading.

@thasti
Copy link

thasti commented Jan 13, 2025

Ohh I see, I wasn't aware pylint would recognize total in your example as a constant as well - in this sense it's at least consistent now (even though counterintuitive). I agree then that this is a more general discussion to be had in a separate issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Invalid Not a bug, already exists or already fixed
Projects
None yet
Development

No branches or pull requests

3 participants