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

Add new checker useless-return #1821

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@atodorov
Contributor

atodorov commented Jan 5, 2018

warns about "return" and "return None" statements at the end of functions/methods. These statements are useless because None is the default return type if they are missing.

Please comment on this feature because there are 20+ existing test cases that break with it. I'd like to know how to "fix" the failing tests before updating documentation, etc.

Add new checker useless-return
warns about "return" and "return None" statements at the end of
functions/methods
@PCManticore

This comment has been minimized.

Show comment
Hide comment
@PCManticore

PCManticore Jan 5, 2018

Member

This contradicts with the new inconsistent-return-statements check that landed in 1.8: https://pylint.readthedocs.io/en/latest/whatsnew/1.8.html

Member

PCManticore commented Jan 5, 2018

This contradicts with the new inconsistent-return-statements check that landed in 1.8: https://pylint.readthedocs.io/en/latest/whatsnew/1.8.html

@PCManticore PCManticore closed this Jan 5, 2018

@atodorov

This comment has been minimized.

Show comment
Hide comment
@atodorov

atodorov Jan 5, 2018

Contributor

@PCManticore

the inconsistent-return-statements says

                  'According to PEP8, if any return statement returns an expression, '
                  'any return statements where no value is returned should explicitly '
                  'state this as return None, and an explicit return statement '
                  'should be present at the end of the function (if reachable)'

however that leaves out the case where there is only one return statement at the end of the function body and it is either "return" or "return None". I am currently working with a legacy codebase which has many of these. For example

def add_cases(run_ids, case_ids):
    """
    trs = TestRun.objects.filter(run_id__in=pre_process_ids(run_ids))
    tcs = TestCase.objects.filter(case_id__in=pre_process_ids(case_ids))

    for tr in trs.iterator():
        for tc in tcs.iterator():
            tr.add_case_run(case=tc)

    return

IMO this checker can still be beneficial to people, but probably needs to be better integrated with the inconsistent-return-statements checker. What do you think ?

Contributor

atodorov commented Jan 5, 2018

@PCManticore

the inconsistent-return-statements says

                  'According to PEP8, if any return statement returns an expression, '
                  'any return statements where no value is returned should explicitly '
                  'state this as return None, and an explicit return statement '
                  'should be present at the end of the function (if reachable)'

however that leaves out the case where there is only one return statement at the end of the function body and it is either "return" or "return None". I am currently working with a legacy codebase which has many of these. For example

def add_cases(run_ids, case_ids):
    """
    trs = TestRun.objects.filter(run_id__in=pre_process_ids(run_ids))
    tcs = TestCase.objects.filter(case_id__in=pre_process_ids(case_ids))

    for tr in trs.iterator():
        for tc in tcs.iterator():
            tr.add_case_run(case=tc)

    return

IMO this checker can still be beneficial to people, but probably needs to be better integrated with the inconsistent-return-statements checker. What do you think ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment