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

Allow opting out of UnreferencedShape? #2093

Closed
kubukoz opened this issue Jan 8, 2024 · 1 comment · Fixed by #2119
Closed

Allow opting out of UnreferencedShape? #2093

kubukoz opened this issue Jan 8, 2024 · 1 comment · Fixed by #2119
Labels
feature-request A feature should be added or improved.

Comments

@kubukoz
Copy link
Contributor

kubukoz commented Jan 8, 2024

Hi!

In our internal Smithy codebase (or should I say, specbase?), we have a bunch of entities that aren't connected to services - e.g. event or Databricks table definitions.

Currently, users have to annotate them with suppressions, which is far from an ideal experience.

Instead, it would be great if we could opt out of the default behavior (in both the LSP and our model assembler) and replace it with custom validation, such as something that'd check each shape is connected to one of these top-level entities.

For example:

metadata disableUnreferencedShapeValidation = true

Alternatively, an ability to expand the list of "top-level things" from "services" to other things (e.g. using a selector):

metadata unreferencedShapeValidation = {
  validationRootSelector: ":is(service, [trait|myEventTrait])"
}

Please consider this enhancement and let me know what you think!

@sugmanue sugmanue added the feature-request A feature should be added or improved. label Jan 9, 2024
@mtdowling
Copy link
Member

mtdowling commented Jan 12, 2024

Yeah, I kinda think we should completely deprecate UnreferencedShapes as a built-in validator, and turn it into something your can opt-into as a configurable linter using some kind of selector approach like you're suggesting.

mtdowling added a commit that referenced this issue Jan 30, 2024
UnreferencedShape is no longer an on-by-default validator that ensures
shapes must be connected to a service shape. There are plenty of cases
where teams might create models that define shared shapes and don't
necessarily connect those shapes to services. Emitting an
UnreferencedShape validation event for these cases is harmless, but
also annoying and desensitizing developers for when a shape is actually
unreferenced.

This commit instead adds a new smithy-linters validator called
UnreferencedShape that allows an optional "rootShapeSelector" to be
provided that defines the "root" shapes that all other shape must be
connected to. It defaults to "service". You might want to customize
this to indicate that other shapes are your roots (e.g., maybe there
is a specific trait that makes a shape a root).

Closes #2093
mtdowling added a commit that referenced this issue Jan 31, 2024
UnreferencedShape is no longer an on-by-default validator that ensures
shapes must be connected to a service shape. There are plenty of cases
where teams might create models that define shared shapes and don't
necessarily connect those shapes to services. Emitting an
UnreferencedShape validation event for these cases is harmless, but
also annoying and desensitizing developers for when a shape is actually
unreferenced.

This commit instead adds a new smithy-model linter called
UnreferencedShape that allows an optional "rootShapeSelector" to be
provided that defines the "root" shapes that all other shape must be
connected to. It defaults to "service". You might want to customize
this to indicate that other shapes are your roots (e.g., maybe there
is a specific trait that makes a shape a root).

Closes #2093
mtdowling added a commit that referenced this issue Feb 8, 2024
UnreferencedShape is no longer an on-by-default validator that ensures
shapes must be connected to a service shape. There are plenty of cases
where teams might create models that define shared shapes and don't
necessarily connect those shapes to services. Emitting an
UnreferencedShape validation event for these cases is harmless, but
also annoying and desensitizing developers for when a shape is actually
unreferenced.

This commit instead adds a new smithy-model linter called
UnreferencedShape that allows an optional "rootShapeSelector" to be
provided that defines the "root" shapes that all other shape must be
connected to. It defaults to "service". You might want to customize
this to indicate that other shapes are your roots (e.g., maybe there
is a specific trait that makes a shape a root).

Closes #2093
hpmellema pushed a commit to hpmellema/smithy that referenced this issue Feb 22, 2024
UnreferencedShape is no longer an on-by-default validator that ensures
shapes must be connected to a service shape. There are plenty of cases
where teams might create models that define shared shapes and don't
necessarily connect those shapes to services. Emitting an
UnreferencedShape validation event for these cases is harmless, but
also annoying and desensitizing developers for when a shape is actually
unreferenced.

This commit instead adds a new smithy-model linter called
UnreferencedShape that allows an optional "rootShapeSelector" to be
provided that defines the "root" shapes that all other shape must be
connected to. It defaults to "service". You might want to customize
this to indicate that other shapes are your roots (e.g., maybe there
is a specific trait that makes a shape a root).

Closes smithy-lang#2093
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants