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 check: invalid-length-returned #845

Merged
merged 1 commit into from
Mar 16, 2016

Conversation

rogalski
Copy link
Contributor

Implementation of issue #557.

@@ -995,16 +995,22 @@ class SpecialMethodsChecker(BaseChecker):
'invalid number of parameters. If it has too few or '
'too many, it might not work at all.',
{'old_names': [('E0235', 'bad-context-manager')]}),
'E0303': ('__len__ does not return non-negative integer',
'bad-len-returned',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you rename this to negative-len-returned?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like saying "negative int" when returned value may be None or not integer at all. Also I've noticed issues like #583, #584, #585, #560 - let's say they will be implemented, should / shouldn't they share error code, like some generic "invalid-type-returned"?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They will probably have different names, so that they can be easily disabled on a case by case basis. How about invalid-length-returned, does that sound better?

@PCManticore
Copy link
Contributor

Thank you for this PR! looks good, I left a couple of comments with improvements that you can do.

@rogalski
Copy link
Contributor Author

Thanks. Do you mind merging multiple commits or do you prefer having them squashed to single one?

@PCManticore
Copy link
Contributor

I don't mind having multiple commits, as long as they have good messages.

@rogalski rogalski changed the title New check: bad-len-returned New check: invalid-length-returned Mar 11, 2016
infered = _safe_infer_call_result(node, node)
if infered is None or infered is astroid.util.Uninferable:
return
value = infered.value
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

infered might not always have a value, no? We can't guarantee it's always a Const value. Can you do add a test for this?

PCManticore added a commit that referenced this pull request Mar 16, 2016
New check: invalid-length-returned
@PCManticore PCManticore merged commit b22b702 into pylint-dev:master Mar 16, 2016
@rogalski rogalski deleted the bad-len-returned branch March 17, 2016 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants