Astroid error on 2.14.3 #6986
Labels
Crash 💥
A bug that makes pylint crash
Downstream Bug 🪲
The problem happens in a lib depending on pylint, not pylint
Waiting on author
Indicate that maintainers are waiting for a message of the author
Milestone
Comments
Meemaw
added
the
Needs triage 📥
|
Could you share the |
Pierre-Sassoulas
added
Crash 💥
jacobtylerwalls
added
Waiting on author
|
Failing line: and next(
str(k.value.value) for k in getattr(node, "keywords", node) if getattr(k, "arg", None) == "model_name"
)Full file: """Migrations checkers for CompanyX API."""
from __future__ import annotations
import os
import sys
from typing import TYPE_CHECKING, Any
import astroid
from astroid import nodes
from pylint.checkers import BaseChecker
from pylint.checkers.utils import check_messages
from pylint.interfaces import IAstroidChecker
if TYPE_CHECKING:
from pylint.lint import PyLinter
### migrations checker ###
# Structure for ALLOW_IMPORT dict:
# "directory name" -> [RECURSE, allowed1, allowed2, ...]
#
# RECURSE is a dict with the same structure:
# "subdir name" -> [RECURSE, allowed1, allowed2, ...]
ALLOW_IMPORT = {
"api": [
{
"lib": [
{
"models": [{}, "base_model"],
"moonpay": [{}, "types"],
"utils": [{}, "string_utils"],
},
"not-allowed",
],
"model": [{}, "fields"],
},
"managers",
"types",
],
"core": [
{},
"blockchain",
"types",
],
"contrib": [
{},
"not-allowed",
],
"dal": [
{},
"not-allowed",
],
"src": [
{},
"not-allowed",
],
}
MIGRATIONS_PREFIX = "api.migrations."
MSGS = {
"E0901": (
"Migration import %s not allowed",
"invalid-migration-import",
"Used when api import detected for migrations.",
),
"E0902": (
"Tables %s and %s should be in different migrations",
"multiple-tables-in-migration",
"Used when multiple tables are updated in the same migration.",
),
"E0903": (
"Index for table %s must use operations.AddIndexConcurrently",
"index-must-be-concurrent",
"Used when index is does not use operations.AddIndexConcurrently.",
),
"E0904": (
"Index for table %s must use migrations.SeparateDatabaseAndState",
"index-must-separate-state",
"Used when index is does not use migrations.SeparateDatabaseAndState.",
),
}
class MigrationsChecker(BaseChecker):
"""Checker for CompanyX migrations
Checks migration dependencies on api except for:
* api.lib.models.base_model
* api.lib.moonpay.types
* api.lib.utils.string_utils
* api.managers
* api.model.fields
* api.types
* core.blockchain
* core.types
"""
__implements__ = IAstroidChecker
name = "migrations"
msgs = MSGS
created_models = []
current_model_name = None
current_root_name = None
model_managers = []
@check_messages("multiple-tables-in-migration", "index-must-be-concurrent", "index-must-separate-state")
def visit_call(self, node: nodes.Call) -> None:
"""Triggered when a function call is seen."""
root_name = node.root().name
for exempt_migration in [
"0001_squashed",
"0680_alter_assetevent_event_type",
"0682_create_twitter_metadata_table",
"0694_remove_assetcontract_api_assetco",
]:
if exempt_migration in root_name:
# skip multiple table checks for squashed migrations
return
if not (root_name.startswith(MIGRATIONS_PREFIX)):
# skip checks for non-migration nodes
return
if (
getattr(node, "parent", None)
and type(node.parent).__name__ == "Keyword"
and getattr(node.parent, "arg", "") in ["index", "constraint"]
):
# skip checks for index and constraint names
return
if self.current_root_name != root_name:
# initialize checks for migration file
self.created_models = []
self.current_model_name = None
self.current_root_name = root_name
self.model_managers = []
function_name = getattr(node.func, "attrname", getattr(node.func, "name", str(node.func)))
self._check_multiple_tables(node, function_name)
if (
self.current_model_name
and "index" in function_name.lower()
and next(
str(k.value.value) for k in getattr(node, "keywords", node) if getattr(k, "arg", None) == "model_name"
)
):
self._check_add_index(node, function_name)
def _check_add_index(self, node: nodes.Call, function_name: str) -> None:
root_name = node.root().name
if "AddIndexConcurrently" != function_name:
self.add_message(
"index-must-be-concurrent",
node=node,
args=(self.current_model_name,),
)
if not (
getattr(node, "parent", None)
and getattr(node.parent, "parent", None)
and type(node.parent.parent).__name__ == "Keyword"
and getattr(node.parent.parent, "arg", "") == "state_operations"
and getattr(node.parent.parent, "parent", None)
and getattr(node.parent.parent.parent, "func", None)
):
parent_func = getattr(node.parent.parent.parent, "func", node.func)
if (
getattr(parent_func, "attrname", None) or getattr(parent_func, "name", None)
) != "SeparateDatabaseAndState":
self.add_message(
"index-must-separate-state",
node=node,
args=(self.current_model_name,),
)
def _check_multiple_tables(self, node: nodes.Call, function_name: str) -> None:
root_name = node.root().name
# table name parameter could be model_name or name
kw_params = [getattr(k, "arg", None) for k in getattr(node, "keywords", node) if getattr(k, "value", None)]
kw_model_name = None
if "model_name" in kw_params:
# If we have keyword model_name, use that value
# Since this is a keyword, there will be only one "model_name"
kw_model_name = next(
str(k.value.value) for k in getattr(node, "keywords", node) if getattr(k, "arg", None) == "model_name"
)
elif "name" in kw_params:
# If we have keyword name but no model_name, use keyword name
# Since this is a keyword, there will be only one "name"
kw_model_name = next(
str(k.value.value) for k in getattr(node, "keywords", node) if getattr(k, "arg", None) == "name"
)
else:
return
if not self.current_model_name:
if function_name == "CreateModel" and not self.model_managers:
# allow creating multiple tables in one migration
self.created_models.append(str(kw_model_name).lower())
return
elif function_name == "AlterModelManagers" and not self.created_models:
# allow multiple managers in one migration
self.model_managers.append(str(kw_model_name).lower())
return
elif self.model_managers:
# Do not allow other migrations with model managers
self.current_model_name = "has-model-managers"
elif self.created_models and kw_model_name not in self.created_models:
# when creating table, only allow adding indexes for created tables
self.current_model_name = self.created_models[0]
if not self.current_model_name:
self.current_model_name = kw_model_name
if kw_model_name != self.current_model_name:
self.add_message(
"multiple-tables-in-migration",
node=node,
args=(str(kw_model_name), str(self.current_model_name)),
)
@check_messages("invalid-migration-import")
def visit_import(self, node: nodes.Import) -> None:
"""Triggered when an import statement is seen."""
self._disallow_migration_imports(node)
@check_messages("invalid-migration-import")
def visit_importfrom(self, node: nodes.ImportFrom) -> None:
"""Triggered when a from statement is seen."""
self._disallow_migration_imports(node)
def _disallow_migration_imports(self, node: nodes.Import | nodes.ImportFrom):
"""Disallow migration imports from api
* exceptions are at ALLOW_IMPORT
"""
if not (node.root().name.startswith(MIGRATIONS_PREFIX)):
# skip import checks for non-migration nodes
return
# Loop over imports for migrations
for name in node.names:
if name and name[0]:
split_name: list[str] = name[0].split(".")
if split_name[0] in ALLOW_IMPORT:
self._check_migration_imports(node, ALLOW_IMPORT, name[0], split_name)
def _check_migration_imports(
self, node: nodes.Import | nodes.ImportFrom, allow_dict, full_name: str, split_name: list[str]
):
"""Disallow migration imports from api
* exceptions are at ALLOW_IMPORT
"""
if len(split_name) < 2 or split_name[0] not in allow_dict:
self.add_message(
"invalid-migration-import",
node=node,
args=(full_name,),
)
elif split_name[1] not in allow_dict[split_name[0]]:
self._check_migration_imports(node, allow_dict[split_name[0]][0], full_name, split_name[1:])
def register(linter: PyLinter) -> None:
linter.register_checker(MigrationsChecker(linter)) |
If there are no I'm wondering why this was changed between We actually have an open issue about this: #4725 |
|
What would the default value be? I guess a better workaround is to just except the Though, it would be interesting to understand what changed from |
|
Well you're using it in an Yeah, it seems like this is proprietary code so it might be hard to debug this, but for a start you could see which That's at least how I would tackle this: a bit of back and forth with print statements to see which condition of the 3 or 4 that are in the |
DanielNoord
removed
the
Needs triage 📥
jacobtylerwalls
removed
the
Needs astroid update
|
I'm going to close this issue as If any further help is needed, don't hesitate to contact me but this should no longer block us from releasing |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Bug description
After upgrading from
2.13.18-->2.14.3, we're seeing some parsing pylint errors.Configuration
Command used
Pylint output
Expected behavior
Pylint works
Pylint version
OS / Environment
No response
Additional dependencies
No response
The text was updated successfully, but these errors were encountered: