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
Use tostr on prop/form names to avoid attribute errors (SYN-6657) #3584
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3584 +/- ##
==========================================
- Coverage 97.66% 97.56% -0.11%
==========================================
Files 234 234
Lines 49737 49755 +18
==========================================
- Hits 48574 48541 -33
- Misses 1163 1214 +51
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
synapse/lib/ast.py
Outdated
|
||
ispiv = name.find('::') != -1 | ||
if not ispiv: | ||
|
||
prop = path.node.form.props.get(name) | ||
if prop is None: | ||
if not isinstance(propname, str): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we functionalize this block to minimize copy/pasta?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a typeerr
function to stormtypes, open to other naming suggestions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think typeerr
is fine, but I could also see it being something like reqType
since it will raise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I initially had it as reqtype
, but it doesn't raise the exception itself, it returns it so we can add the relevant AST info here.
No description provided.