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

Scope tests #2053

Merged
merged 76 commits into from
Dec 6, 2023
Merged
Show file tree
Hide file tree
Changes from 74 commits
Commits
Show all changes
76 commits
Select commit Hold shift + click to select a range
6ac7ea6
Started working on scope tests
AndreasArvidsson Nov 22, 2023
9bc6d8a
Extend scope provider interface
AndreasArvidsson Nov 22, 2023
8326276
Add insertion delimiter
AndreasArvidsson Nov 22, 2023
5feb0cf
Added additional test
AndreasArvidsson Nov 22, 2023
6cd7cc0
[pre-commit.ci lite] apply automatic fixes
pre-commit-ci-lite[bot] Nov 22, 2023
eac4b30
Trailing newline
AndreasArvidsson Nov 22, 2023
fcef5ec
Merge branch 'scope_tests' of github.com:cursorless-dev/cursorless in…
AndreasArvidsson Nov 22, 2023
0348ae4
Added example with multipole scopes
AndreasArvidsson Nov 22, 2023
dde8a77
Added function
AndreasArvidsson Nov 22, 2023
9f79367
Changed representation
AndreasArvidsson Nov 22, 2023
33263bb
[pre-commit.ci lite] apply automatic fixes
pre-commit-ci-lite[bot] Nov 22, 2023
91a5fc2
Change file ending
AndreasArvidsson Nov 22, 2023
9123d4b
[pre-commit.ci lite] apply automatic fixes
pre-commit-ci-lite[bot] Nov 22, 2023
ec2004c
number
AndreasArvidsson Nov 22, 2023
a1e62ea
Merge branch 'scope_t
AndreasArvidsson Nov 22, 2023
fcefb46
fix
AndreasArvidsson Nov 22, 2023
191454f
[pre-commit.ci lite] apply automatic fixes
pre-commit-ci-lite[bot] Nov 22, 2023
5814cd5
test
AndreasArvidsson Nov 22, 2023
e3753b3
fix
AndreasArvidsson Nov 22, 2023
7dcde95
Added paragraph
AndreasArvidsson Nov 22, 2023
c5d8fdb
fix
AndreasArvidsson Nov 22, 2023
071e815
try
AndreasArvidsson Nov 22, 2023
c6b8aaf
fix
AndreasArvidsson Nov 22, 2023
b756723
New format
AndreasArvidsson Nov 22, 2023
9965927
Added facets
AndreasArvidsson Nov 22, 2023
5e76ff0
update
AndreasArvidsson Nov 22, 2023
1995cde
Added language specific scope support facets
AndreasArvidsson Nov 22, 2023
b93893e
[pre-commit.ci lite] apply automatic fixes
pre-commit-ci-lite[bot] Nov 22, 2023
f45a753
clean up
AndreasArvidsson Nov 23, 2023
d5d4c28
clean up
AndreasArvidsson Nov 23, 2023
473a084
cleanup
AndreasArvidsson Nov 23, 2023
ee9fa80
Improve fail message
AndreasArvidsson Nov 23, 2023
2f8dfa2
Support fixture numbers
AndreasArvidsson Nov 23, 2023
bf70406
Made format more compact
AndreasArvidsson Nov 23, 2023
bf5e187
cleanup
AndreasArvidsson Nov 23, 2023
f2b63d8
Use curly braces for empty ranch
AndreasArvidsson Nov 24, 2023
9f9de74
Annotate or trailing whitespace lines
AndreasArvidsson Nov 24, 2023
fc63d43
Extract serialize scopes to own file
AndreasArvidsson Nov 26, 2023
cbd156f
Update line2.scope
pokey Nov 26, 2023
653354e
Update namedFunction.scope
pokey Nov 26, 2023
fee4588
Update name.assignment.scope
pokey Nov 26, 2023
1a96826
Updates scope representation
AndreasArvidsson Nov 27, 2023
0e70883
Add range to removal
AndreasArvidsson Nov 27, 2023
ed21c4d
Update
AndreasArvidsson Nov 27, 2023
f783cb1
Added rest of textual scopes
AndreasArvidsson Nov 27, 2023
4c39f55
Added insertion delimiter
AndreasArvidsson Nov 30, 2023
963675e
Add padding
AndreasArvidsson Nov 30, 2023
c5da832
Minor tweaks
pokey Nov 30, 2023
ef8d452
More tweaks
pokey Nov 30, 2023
66669f2
cleanup
pokey Nov 30, 2023
6c38cc3
rename
pokey Nov 30, 2023
eb348ba
More cleanup
pokey Nov 30, 2023
2f51f65
[pre-commit.ci lite] apply automatic fixes
pre-commit-ci-lite[bot] Nov 30, 2023
459761c
More cleanup
pokey Nov 30, 2023
c096e8a
More tweaks
pokey Nov 30, 2023
ac90914
fix tests
pokey Nov 30, 2023
da04e03
more cleanup
pokey Nov 30, 2023
637d8ea
more cleanup
pokey Nov 30, 2023
aa05a65
Update target number
AndreasArvidsson Nov 30, 2023
4f703a4
Merge branch 'scope_tests' of github.com:cursorless-dev/cursorless in…
AndreasArvidsson Nov 30, 2023
35718a3
use number instead of index
AndreasArvidsson Nov 30, 2023
12e7588
Support subdirectories
AndreasArvidsson Nov 30, 2023
b0f4fef
Updated comment
AndreasArvidsson Nov 30, 2023
cca1d21
Merge branch 'main' into scope_tests
AndreasArvidsson Nov 30, 2023
06d00b4
Added element tags facet
AndreasArvidsson Nov 30, 2023
b8c2ade
Added iteration scope
AndreasArvidsson Dec 1, 2023
5f1d178
Up date javascript support
AndreasArvidsson Dec 1, 2023
93d36d0
Update number serialization
AndreasArvidsson Dec 1, 2023
b8a80a8
Update target number
AndreasArvidsson Dec 2, 2023
7adcef6
Merge branch 'main' into scope_tests
AndreasArvidsson Dec 5, 2023
cfebfd9
cleanup
AndreasArvidsson Dec 5, 2023
72ef3f5
rename
AndreasArvidsson Dec 5, 2023
d427431
Update .pre-commit-config.yaml
AndreasArvidsson Dec 6, 2023
616988e
fix
AndreasArvidsson Dec 6, 2023
2d8c499
Remove unused type
pokey Dec 6, 2023
008f4ce
Remove examples and label
pokey Dec 6, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ repos:
# tests use strings with trailing white space to represent the final
# document contents. For example
# packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/languages/ruby/changeCondition.yml
exclude: ^packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/.*/[^/]*\.yml$|/generated/|^patches/
exclude: ^packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/.*/[^/]*\.yml$|\.scope$|/generated/|^patches/
- repo: local
hooks:
- id: eslint
Expand Down
4 changes: 4 additions & 0 deletions packages/common/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,3 +94,7 @@ export * from "./types/TestCaseFixture";
export * from "./util/getEnvironmentVariableStrict";
export * from "./util/CompositeKeyDefaultMap";
export * from "./util/toPlainObject";
export * from "./scopeSupportFacets/scopeSupportFacets.types";
export * from "./scopeSupportFacets/scopeSupportFacetInfos";
export * from "./scopeSupportFacets/textualScopeSupportFacetInfos";
export * from "./scopeSupportFacets/getLanguageScopeSupport";
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { htmlScopeSupport } from "./html";
import { javascriptScopeSupport } from "./javascript";
import { LanguageScopeSupportFacetMap } from "./scopeSupportFacets.types";

export function getLanguageScopeSupport(
languageId: string,
): LanguageScopeSupportFacetMap {
switch (languageId) {
case "javascript":
return javascriptScopeSupport;
case "html":
return htmlScopeSupport;
}
throw Error(`Unsupported language: '${languageId}'`);
}
19 changes: 19 additions & 0 deletions packages/common/src/scopeSupportFacets/html.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import {
LanguageScopeSupportFacetMap,
ScopeSupportFacetLevel,
} from "./scopeSupportFacets.types";

const { supported, notApplicable } = ScopeSupportFacetLevel;

export const htmlScopeSupport: LanguageScopeSupportFacetMap = {
["key.attribute"]: supported,
["tags"]: supported,

namedFunction: notApplicable,
["name.assignment"]: notApplicable,
["key.mapPair"]: notApplicable,
["key.mapPair.iteration"]: notApplicable,
["value.mapPair"]: notApplicable,
["value.mapPair.iteration"]: notApplicable,
["value.assignment"]: notApplicable,
};
19 changes: 19 additions & 0 deletions packages/common/src/scopeSupportFacets/javascript.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import {
LanguageScopeSupportFacetMap,
ScopeSupportFacetLevel,
} from "./scopeSupportFacets.types";

const { supported, notApplicable } = ScopeSupportFacetLevel;

export const javascriptScopeSupport: LanguageScopeSupportFacetMap = {
namedFunction: supported,
["name.assignment"]: supported,
["key.mapPair"]: supported,
["key.mapPair.iteration"]: supported,
["value.mapPair"]: supported,
["value.mapPair.iteration"]: supported,
["value.assignment"]: supported,

["key.attribute"]: notApplicable,
["tags"]: notApplicable,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this isn't true. JSX is valid in JS. Shall we just mark it unsupported / leave it undefined for now?

Copy link
Member Author

@AndreasArvidsson AndreasArvidsson Dec 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking that we should test all jsx under javascriptreact and not javascript. I would leave it undefined if anything. Saying that it's unsupported is not true either.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. Probably fine for now. At some point we should prob figure out how to automatically run tests for all langs where they're applicable, eg run our js tests in ts, etc. We could maybe make subsets of languages and then define a way to import tests from one language to another. But we can do that in a follow-up

};
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
import {
ScopeSupportFacet,
ScopeSupportFacetInfo,
} from "./scopeSupportFacets.types";

export const scopeSupportFacetInfos: Record<
ScopeSupportFacet,
ScopeSupportFacetInfo
> = {
namedFunction: {
label: "Named function",
description: "A named function",
scopeType: "namedFunction",
examples: ["function foo() {}", "const foo = () => {}"],
},
["name.assignment"]: {
label: "Assignment name",
description: "Name(LHS) of an assignment",
scopeType: "name",
examples: ["const foo = 1"],
},
["key.attribute"]: {
label: "Attributes key",
description: "Key(LHS) of an attribute",
scopeType: "collectionKey",
examples: ['id="root"'],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As in other examples, this would be more helpful if there were a way to put it in context, but a flat string is not a particularly comfortable way to do that

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely.

},
["key.mapPair"]: {
label: "Map key",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These labels feel kinda useless / arbitrary and a bit redundant with the description. I wonder if we just autogenerate these from the id like we do for human-readable scope types. Ie key.mapPair => "key (map pair)"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me

description: "Key(LHS) of a map pair",
scopeType: "collectionKey",
examples: ["value: 0"],
},
["key.mapPair.iteration"]: {
label: "Map pair key iteration",
description: "Iteration of map pair keys",
scopeType: "collectionKey",
isIteration: true,
examples: ["{ value: 0 }"],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't include the curly brackets, but then it becomes pretty unclear what's going on here. I guess we could use multiple?

Copy link
Member Author

@AndreasArvidsson AndreasArvidsson Dec 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The examples are not the content range. It's just example of the type of code that would have this facet

},
["value.assignment"]: {
label: "Assignment value",
description: "Value(RHS) of an assignment",
scopeType: "value",
examples: ["const foo = 1"],
},
["value.mapPair"]: {
label: "Map pair value",
description: "Key(RHS) of a map pair",
scopeType: "value",
examples: ["value: 0"],
},
["value.mapPair.iteration"]: {
label: "Map pair value iteration",
description: "Iteration of map pair values",
scopeType: "value",
isIteration: true,
examples: ["{ value: 0 }"],
},
["tags"]: {
label: "Tags",
description: "Both tags in an xml element",
scopeType: "xmlBothTags",
examples: ["<div></div>"],
},
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
import { SimpleScopeTypeType } from "../types/command/PartialTargetDescriptor.types";

const scopeSupportFacets = [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do feel like all of these could be their own fine-grained scope. I wonder if we think about moving in that direction sooner rather than later

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite follow what you're suggesting? What exactly do you want to make more fine grained? I definitely prefer them fine grained

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eg key.mapPair, key.attribute, etc could all be their own scopes. I think every facet here should probably just be a new fine-grained scope. Not something to change for this PR just an observation. Makes me question the notion of facets entirely, but they're undoubtedly a step forward so nothing to get in the way of this PR

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally agree

// "list",
// "list.interior",
// "map",
// "map.interior",
// "collectionKey",
"namedFunction",
// "namedFunction.interior",
// "functionName",
// "anonymousFunction",
// "anonymousFunction.interior",
"name.assignment",
"key.attribute",
"key.mapPair",
"key.mapPair.iteration",
"value.assignment",
"value.mapPair",
"value.mapPair.iteration",
// "value.assignment.removal",
// "value.return",
// "value.return.removal",
// "value.collectionItem",
// "value.collectionItem.removal",
// "statement",
// "ifStatement",
// "condition.if",
// "condition.while",
// "condition.doWhile",
// "condition.for",
// "condition.ternary",
// "branch",
// "comment.line",
// "comment.block",
// "string.singleLine",
// "string.multiLine",
// "textFragment",
// "functionCall",
// "functionCallee",
// "argumentOrParameter.argument",
// "argumentOrParameter.argument.removal",
// "argumentOrParameter.parameter",
// "argumentOrParameter.parameter.removal",
// "class",
// "class.interior",
// "className",
// "type",
"tags",
] as const;

const textualScopeSupportFacets = [
"character",
"word",
"token",
"line",
"paragraph",
"document",
] as const;

export interface ScopeSupportFacetInfo {
readonly label: string;
readonly description: string;
readonly scopeType: SimpleScopeTypeType;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's going to happen when we want to test "round"? That's not a simple scope type

Copy link
Member Author

@AndreasArvidsson AndreasArvidsson Dec 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just copied the interface from your issue. We can make it a scope type instead?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe ok for the time being. I wonder if we create a utility function to make it more ergonomic once we need to support complex scopes.

readonly isIteration?: boolean;
readonly examples: readonly string[];
}

export enum ScopeSupportFacetLevel {
supported,
unsupported,
notApplicable,
}

export type ScopeSupportFacet = (typeof scopeSupportFacets)[number];

export type TextualScopeSupportFacet =
(typeof textualScopeSupportFacets)[number];

export type LanguageScopeSupportFacetMap = Partial<
Record<ScopeSupportFacet, ScopeSupportFacetLevel>
>;

export type TextualLanguageScopeSupportFacetMap = Record<
TextualScopeSupportFacet,
ScopeSupportFacetLevel
>;
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import {
ScopeSupportFacetInfo,
TextualScopeSupportFacet,
} from "./scopeSupportFacets.types";

export const textualScopeSupportFacetInfos: Record<
TextualScopeSupportFacet,
ScopeSupportFacetInfo
> = {
character: {
label: "Character",
description: "A single character in the document",
scopeType: "character",
examples: ["a", "."],
},
word: {
label: "Word",
description: "A single word in a token",
scopeType: "word",
examples: ["foo_bar", "fooBar"],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These examples are pretty misleading. If we're going to take the example thing seriously, we need to figure out a way to demarcate substrings. I do question how useful / ergonomic it is to just drop these examples into strings. The examples from test casees are already there and likely to be far more useful

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking examples as a very general guide. If you want details you she'd look at the actual fixtures

},
token: {
label: "Token",
description: "A single token in the document",
scopeType: "token",
examples: ["foo", "("],
},
line: {
label: "Line",
description: "A single line in the document",
scopeType: "line",
examples: ["foo"],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is unhelpful in the other direction. Really questioning the utility of these examples 🤔

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you don't like them we can't ditch them

},
paragraph: {
label: "Paragraph",
description:
"A single paragraph(contiguous block of lines) in the document",
scopeType: "paragraph",
examples: ["foo\nbar"],
},
document: {
label: "Documents",
description: "The entire document",
scopeType: "document",
examples: ["foo\n\nbar"],
},
};
18 changes: 18 additions & 0 deletions packages/common/src/testUtil/getFixturePaths.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ export function getRecordedTestsDirPath() {
return path.join(getFixturesPath(), "recorded");
}

export function getScopeTestsDirPath() {
return path.join(getFixturesPath(), "scopes");
}

export function getRecordedTestPaths() {
const directory = getRecordedTestsDirPath();
const relativeDir = path.dirname(directory);
Expand All @@ -34,3 +38,17 @@ export function getRecordedTestPaths() {
path: p,
}));
}

export function getScopeTestPaths() {
const directory = getScopeTestsDirPath();
const relativeDir = path.dirname(directory);

return walkFilesSync(directory)
.filter((p) => p.endsWith(".scope"))
.map((p) => ({
path: p,
name: path.relative(relativeDir, p.substring(0, p.lastIndexOf("."))),
languageId: path.dirname(path.relative(directory, p)).split(path.sep)[0],
facet: path.basename(p).match(/([a-zA-Z.]+)\d*\.scope/)![1],
}));
}
6 changes: 6 additions & 0 deletions packages/common/src/types/ScopeProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,13 @@ export interface ScopeRanges {
*/
export interface TargetRanges {
contentRange: Range;
removalRange: Range;
removalHighlightRange: GeneralizedRange;
leadingDelimiter: TargetRanges | undefined;
trailingDelimiter: TargetRanges | undefined;
interior: TargetRanges[] | undefined;
boundary: TargetRanges[] | undefined;
insertionDelimiter: string;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,36 @@ import {
import { Target } from "../typings/target.types";

export function getTargetRanges(target: Target): TargetRanges {
const interior = (() => {
try {
target.getInteriorStrict().map(getTargetRanges);
} catch (error) {
return undefined;
}
})();

const boundary = (() => {
try {
target.getBoundaryStrict().map(getTargetRanges);
} catch (error) {
return undefined;
}
})();

return {
contentRange: target.contentRange,
removalRange: target.getRemovalRange(),
removalHighlightRange: target.isLine
? toLineRange(target.getRemovalHighlightRange())
: toCharacterRange(target.getRemovalHighlightRange()),
leadingDelimiter: getOptionalTarget(target.getLeadingDelimiterTarget()),
trailingDelimiter: getOptionalTarget(target.getTrailingDelimiterTarget()),
interior,
boundary,
insertionDelimiter: target.insertionDelimiter,
};
}

function getOptionalTarget(target: Target | undefined) {
return target != null ? getTargetRanges(target) : undefined;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<div id="root"></div>
---

[Content] = 0:5-0:7
0| <div id="root"></div>
>--<

[Removal] = 0:5-0:8
0| <div id="root"></div>
>---<

[Trailing delimiter] = 0:7-0:8
0| <div id="root"></div>
>-<

[Domain] = 0:5-0:14
0| <div id="root"></div>
>---------<

[Insertion delimiter] = " "
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<div>hello</div>
---

[.1 Content] =
[.1 Removal] = 0:0-0:5
0| <div>hello</div>
>-----<

[.1 Insertion delimiter] = " "

[.2 Content] =
[.2 Removal] = 0:10-0:16
0| <div>hello</div>
>------<

[.2 Insertion delimiter] = " "

[Domain] = 0:0-0:16
0| <div>hello</div>
>----------------<
Loading