feat(parsing): Allow to infer default scope for markers#2677
feat(parsing): Allow to infer default scope for markers#2677stanislaw merged 1 commit intostrictdoc-project:mainfrom
Conversation
|
There's a trade-off. This raises StrictDocSemanticError as early as possible in MarkerParser.parse and immediately crashes the app, as opposed to deferring scope validation to somewhere in general_language_marker_processors.py.
|
3876cd9 to
6cf2025
Compare
| reqs = ",".join(sorted(used_uids)) | ||
| raise StrictDocSemanticError( | ||
| f"@relation marker for requirements {reqs} misses scope argument.", | ||
| hint="Scope can only be omitted if supported by language, as e.g. with Rust doc comments.", |
There was a problem hiding this comment.
This naturally asks whether we would also do the same for C functions, because there we can also infer the default scope.
There was a problem hiding this comment.
Yes. We already kind of have this, see #2507. As written below, I think for C it would be best to enable auto-scope, but allow to override.
Is there immediate action for this PR required?
| markers.append(line_marker) | ||
| elif relation_scope is None: | ||
| reqs = ",".join(sorted(used_uids)) | ||
| raise StrictDocSemanticError( |
There was a problem hiding this comment.
Con: The reported error message lacks a file name, which would have been a good hint to users wanting to fix the error. Because, MarkerParser.parse currently doesn't know the file name.
I think we should prefer raising here and adding the file name as an argument to marker_parser, trading the extra argument in exchange for immediate error reporting.
This is a kind of question when I am not 100% sure but still prefer reporting errors as early as possible over the number of arguments concern.
There was a problem hiding this comment.
Done (as additional parameter).
We could maybe do more refactoring here: MarkerParser has only static methods and no attributes. It's a class without state. And, we have ParseContext, which in my understanding is that missing state handled externally by callers. Couldn't we have something like this:
# move ParseContext to marker_parser.py
class ParseContext:
...
class MarkerParser:
def __init__(self, file_path, file_stats):
self.context = ParseContext(file_path, file_stats)
def context():
return self.contextThis would MarkerParser give state as usual in OOP. The state can be used for better error messages inside MarkerParser. And it relieves callers from having to know details about the relation between MarkerParser and ParseContext.
If you like the idea I'll try it, but in a separate PR.
There was a problem hiding this comment.
If you like the idea I'll try it, but in a separate PR.
I like the idea with a small remark that this we should probably not do:
move ParseContext to marker_parser.py
Let's make MarkerParser aware of the ParseContext but not move it because inside all Readers the parse context is used directly without any relation to the marker parser.
Please do it in a separate PR.
(Let's do this in a separate PR.)
WHAT: This allows language readers to provide an inferred default scope to the marker parser. WHY: In some situations, the explicit scope in '@relation(REQ-1, scope=function)' provided by the user would be redundant. A good example are Rust doc comments, which unambiguously determine the targeted language construct. The construct in turn determines the scope. If a language reader has the ability to infer the scope, it should do so and not urge the user to provide it explicitly. This was triggered by strictdoc-project#2672. HOW: We slightly change lark grammar for @relation to: - at least one requirement uid, - then maybe more separated requirement uids, - then maybe separated scope, - then maybe separated role. The default scope is used if the marker contained no user provided scope. We exit with error if neither default nor user provided scope were given.
6cf2025 to
f8f49b6
Compare
WHAT
This allows language readers to provide an inferred default scope to the marker parser.
WHY
In some situations, the explicit scope in
@relation(REQ-1, scope=function)provided by the user would be redundant. A good example are Rust doc comments, which unambiguously determine the targeted language construct. The construct in turn determines the scope. If a language reader has the ability to infer the scope, it should do so and not urge the user to provide it explicitly. This was triggered by #2672.HOW
We slightly change lark grammar for
@relationto:The default scope is used if the marker contained no user provided scope. We exit with error if neither default nor user provided scope were given.