fix(core/webidl): warn when partial interface has local dfn#5242
Open
marcoscaceres wants to merge 3 commits into
Open
fix(core/webidl): warn when partial interface has local dfn#5242marcoscaceres wants to merge 3 commits into
marcoscaceres wants to merge 3 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a new core/webidl authoring warning for cases where a local <dfn> is used for a WebIDL partial definition, aiming to catch specs that redefine interfaces which are actually defined elsewhere.
Changes:
- Adds warning logic in
defineIdlName()for partial IDL definitions that resolve to a local<dfn>withoutdata-cite. - Uses a regex-based scan of local IDL blocks to suppress the warning when a non-partial definition exists in the same spec.
- Adds a spec test covering the new warning behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/core/webidl.js |
Adds the new partial-definition warning and local non-partial detection logic. |
tests/spec/core/webidl-spec.js |
Adds a regression test asserting that the new warning is emitted. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
marcoscaceres
force-pushed
the
fix/3563-partial-warning
branch
2 times, most recently
from
9c95265 to
c1c071e
Compare
Contributor
Author
|
@copilot Can you confirm all your feedback has been addressed? Can you fix anything that was not addressed? |
Contributor
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Contributor
Author
|
@copilot Can you confirm all your feedback has been addressed? Can you fix anything that was not addressed? |
Contributor
All three items of feedback are addressed in commit d7018ec:
|
…ata-cite Checks raw IDL source for non-partial definitions to avoid false positives. Only warns when no non-partial definition exists. Closes #3563
…heck The regex for detecting non-partial base definitions required the declaration keyword at start-of-line, missing same-line extended attributes like `[Exposed=Window] interface Foo`. Now allows an optional `]` before the keyword. Add 3 regression tests: non-partial base exists (no warning), data-cite dfn (no warning), extended-attribute-prefixed base (no warning).
marcoscaceres
force-pushed
the
fix/3563-partial-warning
branch
from
d7018ec to
7f2c792
Compare
Contributor
Author
|
@copilot Can you confirm all your feedback has been addressed? Can you fix anything that was not addressed? |
…al check Agent-Logs-Url: https://github.com/speced/respec/sessions/56153fca-bad2-4ace-a20e-628ca0d9e7cd Co-authored-by: marcoscaceres <870154+marcoscaceres@users.noreply.github.com>
Contributor
All original feedback is addressed. I also found and fixed one additional issue: the extended-attribute test was named "warns for partial with extended-attribute-prefixed base" but was actually asserting zero warnings — corrected to "does not warn when non-partial base has same-line extended attributes" in commit 5cd835a. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3563