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

WIP: Add invalid-envvar checkers #1680

Closed
wants to merge 4 commits into from

Conversation

renatgalimov
Copy link
Contributor

@renatgalimov renatgalimov commented Sep 27, 2017

Features

  • Added checking variable types for getenv functions

For the ticket #1669

@renatgalimov renatgalimov changed the title WIP: Added support for getenv function WIP: Add invalid-envvar checkers Sep 27, 2017
Copy link
Contributor

@PCManticore PCManticore left a comment

Choose a reason for hiding this comment

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

The initial effort is pretty good! There are a couple of changes that we need though

@@ -0,0 +1,5 @@
import os
Copy link
Contributor

Choose a reason for hiding this comment

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

Take a look at the other test files from the functional directory. The .txt should be a mapping of the error occurrences from the .py file.

@@ -287,6 +317,42 @@ def _check_open_mode(self, node):
self.add_message('bad-open-mode', node=node,
args=mode_arg.value)

def _check_invalid_envvar_value(self, call_arg, infer):
if isinstance(call_arg, Call):
Copy link
Contributor

Choose a reason for hiding this comment

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

We have an utility called safe_infer which you should use instead.

return

if isinstance(call_arg, Const):
if not isinstance(call_arg.value, (six.string_types, six.text_type)):
Copy link
Contributor

Choose a reason for hiding this comment

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

six.string_types should be enough since it will contain both unicode and str on Python 2.


if isinstance(call_arg, Const):
if not isinstance(call_arg.value, (six.string_types, six.text_type)):
self.add_message('invalid-envvar-value', node=call_arg, args=(infer.qname(), call_arg.pytype()))
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be the node that you are checking against, not the inferred node (e.g the node from the visit_ function)

@renatgalimov
Copy link
Contributor Author

Guys, could you please tell how to generate the test/functional/*.txt file? My message format is different than that one which you use.

@rogalski
Copy link
Contributor

rogalski commented Nov 1, 2017

@femdom Hello, and sorry for late response.

Parametrized test runner (test_functional function in pylint.test.test_functional module) exposes two usage models, where one (LintModuleTest) runs standard assertions and second (LintModuleOutputUpdate) updates expected output files.

@PCManticore
Copy link
Contributor

@femdom Let us know if you need any help generating that file! Other than that, this PR seems close to being merged

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@PCManticore
Copy link
Contributor

@femdom I updated the tests and merged this locally. Thank you so much for the contribution and sorry for the huge delay getting this in!

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

Successfully merging this pull request may close these issues.

None yet

3 participants