feat(core): support scoped aliases #1840
Conversation
P0lip
force-pushed
the
feat/core/support-scoped-aliases
branch
3 times, most recently
from
c2f3330
to
bc92140
Compare
P0lip
changed the base branch from
develop
to
chore/core/scoped-aliases-validation
P0lip
force-pushed
the
feat/core/support-scoped-aliases
branch
from
bc92140
to
d5353da
Compare
| { targets }: RulesetScopedAliasDefinition, | ||
| formats: Set<Format> | null, | ||
| ): string { | ||
| const errorMessage = `Alias "${name}" is applicable to certain formats, but the format of the linted document is not matched`; |
There was a problem hiding this comment.
How does this manifest itself to end user, and what behavior does it cause? If there's a test around this behavior lmk and that might tell me, just couldn't find it :).
If there is a rule that uses an alias that isn't applicable to the document being linted, will this cause the lint to not go through and/or error?
There was a problem hiding this comment.
given the following document
swagger: 2.0
definitions:
User:
type: woopsand the following ruleset
formats:
- oas2
- oas3
aliases:
SchemaObject:
targets:
- formats:
- oas3
given: $.components.schemas[*]
rules:
valid-type-schema:
given: "#SchemaObject"
then:
field: type
function: enumeration
functionOptions:
values:
- string
- boolean
- number
- null
- object
- arrayThere was a problem hiding this comment.
I can't quite tell from the screenshot - did that result in the lint failing, or is it just a log to stdout?
There was a problem hiding this comment.
@marbemac it fails, I updated the screenshot.
There was a problem hiding this comment.
Hmm - do we want that? makes it impossible to write some rules that target oas2 and some that target oas3 etc
There was a problem hiding this comment.
But we don't inform the user about a potentially broken json path
We don't, but it's more apparent than it is with aliases.
I agree with the "pointer to a json path" bit.
I mean, I'm certainly not against making it more lenient - just need to evaluate how this would feel.
Let's make Nauman decide.
Getting rid of that throw would be very easy, so let's not take the implementation factor into the account when making the choice.
Both options are easy to implement.
There was a problem hiding this comment.
After some thinking, I actually believe that the case we deal with here might closer to to an empty given:
given: []
There was a problem hiding this comment.
I think letting the file lint is the right thing to do. We should have some sort of warning though that a particular rule wasn't run as folks could assume that the file was linted against that rule. Maybe that as a message/info is the right way to go about it?
There was a problem hiding this comment.
That could be a separate thing though. Doesn't need to happen in this PR
There was a problem hiding this comment.
Fair enough.
I'd still argue we should throw, but let it be. It doesn't change much from the perspective of code.
Yeah, there's some in platform-internal https://github.com/stoplightio/platform-internal/issues/8141
That's actually a difficult question. I'd say it depends. That's why I'm thinking about exposing some internal logic I use in that library so we can potentially use it in Spectral/Studio, and warn about inefficient/bad JSONPath expressions. |
# [@stoplight/spectral-core-v1.6.0](https://github.com/stoplightio/spectral/compare/@stoplight/spectral-core-v1.5.1...@stoplight/spectral-core-v1.6.0) (2021-10-12) ### Features * **core:** support scoped aliases ([#1840](#1840)) ([b278497](b278497))
|
🎉 This PR is included in version @stoplight/spectral-core-v1.6.0 🎉 The release is available on npm package (@latest dist-tag) Your semantic-release bot 📦🚀 |
Closes #1837
Needs #1838 #1839
Checklist
Does this PR introduce a breaking change?