Repository navigation
feat: warn about properties an element doesn't have in views and stylesheets - #67
Merged
Merged
Conversation
… have Alloy is silent on this. `widht: 10` in a `.tss` and `colr="red"` in a view both compile without a warning, reach the generated controller verbatim and do nothing at run time. The question is one `@types/titanium` already answers, so there is no table of properties here: `core/cascade.ts` says which rules reach which element and what each sets, `ProjectService.membersOf` says what that element's type has, and this is the comparison. A pure function of the views and rules it is given, so the same call serves the server, a build and a test. Three rules decide a finding, and each is a false positive otherwise. Measured against tidev/kitchensink-v2 — 78 views, 999 rules, 567 elements — they take it from 18 findings to 3, and all three stand up: - Reported once per place it is written, not once per element it reaches. A `"Label"` rule in a view of nine labels is one authored mistake. - A rule is judged against every element it styles together, so a class shared by a Label and a Window keeps `text` for one and `orientationModes` for the other. Unknown means unknown to all of them. - An element the project has no types for silences the rule it is in. A native module's proxies are not in the package at all, and nothing is what we know. Two kinds of element need Alloy's own rules rather than the types: - Anything inside abstract markup is an entry of a property rather than a proxy. `<ButtonBar>` translates `<Labels>` to `BarItemTypes` and its children become the `labels` array, where `image` belongs to an entry and not to a Label. Twelve of the eighteen were this. - `Ti.UI.Toolbar.js` hands `displayHomeAsUp` and its siblings to the activity when a Window names the toolbar as its ActionBar, and reads `customToolbar` off that Window. Alloy passes all of them to the proxy, so the cascade is right to report them and they are still not mistakes. What survives on KitchenSink is one real bug — `mininum` for `minimum` on a Stepper, so the minimum is never set — one property the package types as `never` on `Ti.UI.Android.CardView`, and one it does not carry at all. `isAbstractTag` is lifted out of `startsLocal`, which already had the test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QY3KqD5KAhfTKWFNTiEFGB
Checking a stylesheet asks after the same few types once per element per property. On tidev/kitchensink-v2 that is 4467 calls for 50 types in app.tss alone, and each one rendered every member's type and documentation from scratch: 3.9s for one file. membersOf now answers from memory until the program changes, keyed on the program itself so an edit that could change a type is never answered stale. The list is shared, so it is read only. Refs #66 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AAKDFVwAZoxefLNvX45pE4
…fix it findingsInView checks a view's own attributes. findingsInStyle checks a stylesheet against every view it styles at once, because a rule is judged against all its elements together: app.tss against every view, widgets included, and a theme no build selects against nothing. Each check asks a type about its members once rather than once per property. Even a source that remembers its answers has to find out whether its program has changed first, and within one check it has not. Together with the cache in the host, KitchenSink's app.tss takes 170ms rather than 3.9s, and every other file a few milliseconds. The findings are the same three. messageFor names the types as Titanium code writes them, and replacementFor gives the edit for a suggestion: the last segment of the name, inside any quotes, checked against the text rather than trusted. Refs #66 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AAKDFVwAZoxefLNvX45pE4
Every open view and stylesheet is checked once typing stops for 300ms, rather than only the one that changed: what a stylesheet gets wrong depends on the views it styles, so an edit to one can add or take away a warning in the other. A document that changed or closed while it was being checked is left alone, since another check is already queued. Closing a document takes its warnings back. The quick fix is worked out again from the file rather than read back from the diagnostic, because carrying data on a diagnostic needs a capability not every client has. It names the warning it answers when the client sends it. There is no setting to turn this off. The server has none by design (#14), and every editor already has its own controls for a language server's diagnostics. Refs #66 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AAKDFVwAZoxefLNvX45pE4
Refs #66 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AAKDFVwAZoxefLNvX45pE4
DefinitelyTyped published no @types/titanium for SDK 10 or 11, so a project on either resolves to 9.x. That is safe for completion and hover, which only offer what the types know, and wrong for a check that a name exists: every property those SDKs added is real and reads as unknown, and the quick fix suggests a name that is wrong. So a project whose types are an older major than its SDK is told why once in the log and warned about nothing. Wherever the types came from, the project's own node_modules included. A release that trails inside the SDK's own major is still checked, since nearly every project is in that position and the members a minor adds are few. Every Alloy fixture was in that position itself, on SDK 10 or 12 against the 9.2.2 stub, so the tests of the warnings now lend the stub as a 10.x release, and the stdio test uses a new fixture on SDK 9. Refs #66 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AAKDFVwAZoxefLNvX45pE4
There was a problem hiding this comment.
🟡 Changes recommended
Untyped targets and shared-rule aggregation produce false warnings, while nested-path handling suppresses valid findings.
4 open findings
What changed in this PR
Adds the server’s first diagnostics: warnings and spelling fixes for unknown properties in Alloy views and stylesheets.
Changes:
- Adds property checking, nested-path analysis, and typo suggestions.
- Publishes debounced diagnostics and offers quick fixes.
- Caches type members and adds core, adapter, and protocol tests.
| File | Description |
|---|---|
| src/test/server/service.test.ts | Tests diagnostics and quick fixes. |
| src/test/server/server.test.ts | Tests warning publication over stdio. |
| src/test/server/lsp-client.ts | Adds notification waiting. |
| src/test/server/fake-connection.ts | Captures diagnostics and supports fix requests. |
| src/test/fixtures/alloy-matched-project/tiapp.xml | Defines a matching-major SDK fixture. |
| src/test/fixtures/alloy-matched-project/node_modules/@types/titanium/package.json | Links fixture stub types. |
| src/test/fixtures/alloy-matched-project/app/views/index.xml | Adds a minimal fixture view. |
| src/test/fixtures/alloy-matched-project/app/styles/app.tss | Adds fixture global styles. |
| src/test/fixtures/alloy-matched-project/app/config.json | Defines fixture Alloy configuration. |
| src/test/core/typescript/versions.test.ts | Tests SDK compatibility gating. |
| src/test/core/typescript/host.test.ts | Tests member-cache reuse and invalidation. |
| src/test/core/registry.test.ts | Includes the new fixture. |
| src/test/core/check.test.ts | Tests property checking and replacements. |
| src/server/service.ts | Schedules diagnostics and builds quick fixes. |
| src/server/convert.ts | Converts findings into protocol warnings. |
| src/core/view.ts | Allows readonly member results. |
| src/core/typescript/versions.ts | Adds types-to-SDK compatibility checks. |
| src/core/typescript/host.ts | Caches members per TypeScript program. |
| src/core/tags.ts | Exposes abstract-tag detection. |
| src/core/check.ts | Implements property analysis and suggestions. |
| README.md | Documents warnings and limitations. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…ually says Review on #67 found four ways a rule shared between elements, or a nested property, was judged wrongly: - <Require> and <Widget> have no Titanium type, so the cascade leaves them out, and a class they share with a Label was checked against the Label alone. They are now in the rule's reach and, like a native module, silence it. - <Module module="ti.map"> resolves to Ti.UI.View, and so does any tag with a module attribute, though a native module creates them. Both are now opaque. An ns attribute is not: ns="Ti.UI.iOS" names a Titanium type, and an ns that names a module already resolves to a type the project does not have. - A property Alloy reads for itself returned early, so it never counted as known, and a class shared by a Toolbar and a Label was flagged for displayHomeAsUp. It is now known for the rule. - An empty answer for a nested path meant opaque, so a Window having no font hid a Label's fntSize, and a misspelt outer key such as fnt was never reported. Each type now walks the path and answers known, unknown at a depth, unanswerable (a value that is not an object) or opaque, and only opaque silences a rule. A misspelt outer key is reported once, on that key. KitchenSink is unchanged: the same three findings, and app.tss in 170ms. Refs #66 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AAKDFVwAZoxefLNvX45pE4
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 #66
The server's first diagnostic. Alloy compiles
widht: 10in a.tssfile andcolr="red"on a<Label>without a word, and the property just never applies. Now the editor shows a warning where the property is written, with a quick fix to the name it was probably meant to be.What it reports
app.tss, themes, platform folders and widgets. So a class shared by a<Label>and a<Window>can settextandorientationModeswithout either being flagged.customToolbaron the parent<Window>.font: { fntSize }andfont.fntSize="…"are checked by their path.What it never guesses at
<Require>,<Widget>,<Module>,ns=/module=elements and native modules. A rule that reaches any of them isn't checked.<ButtonBar><Labels><Label image="…"/></Labels></ButtonBar>becomes an entry of thelabelsarray, not aLabel.Delivery
textDocument/publishDiagnosticsas warnings, with sourcetitaniumand codeunknown-property. Alloy projects only.Speed
On
tidev/kitchensink-v2, the uncached check took 3.9s forapp.tss, which styles all 78 views. Two caches bring it to about 170ms, and every other file to a few milliseconds:ProjectService.membersOfkeeps its answers for as long as the TypeScript program lasts.Checked against a real app
tidev/kitchensink-v2@4d43b4c(SDK 13.1.1, real@types/titanium13.3.0, 78 views, 999 rules) gives 3 findings, and each holds up:The first is a real bug in that app. It was 18 findings before the abstract-markup and Alloy-property rules.
Tests
1277 tests pass. Coverage is 99.61% of lines and 97.01% of branches, and lint and the type check are clean.
alloy-matched-project, on SDK 9, because every other Alloy fixture is on SDK 10 or 12 against the 9.2.2 stub types.Out of scope
Each can be its own issue:
color: 42$.id, handlers, translation keys🤖 Generated with Claude Code
https://claude.ai/code/session_01AAKDFVwAZoxefLNvX45pE4
Generated by Claude Code