diff --git a/extensions/ql-vscode/CHANGELOG.md b/extensions/ql-vscode/CHANGELOG.md index 5d605da4e60..03e497d4b06 100644 --- a/extensions/ql-vscode/CHANGELOG.md +++ b/extensions/ql-vscode/CHANGELOG.md @@ -2,6 +2,7 @@ ## [UNRELEASED] +- Added ability to filter repositories for a variant analysis to only those that have results [#2343](https://github.com/github/vscode-codeql/pull/2343) - Add new configuration option to allow downloading databases from http, non-secure servers. [#2332](https://github.com/github/vscode-codeql/pull/2332) ## 1.8.2 - 12 April 2023 diff --git a/extensions/ql-vscode/src/pure/variant-analysis-filter-sort.ts b/extensions/ql-vscode/src/pure/variant-analysis-filter-sort.ts index 93fe0504ce2..0030fd44bfa 100644 --- a/extensions/ql-vscode/src/pure/variant-analysis-filter-sort.ts +++ b/extensions/ql-vscode/src/pure/variant-analysis-filter-sort.ts @@ -3,6 +3,12 @@ import { RepositoryWithMetadata, } from "../variant-analysis/shared/repository"; import { parseDate } from "./date"; +import { assertNever } from "./helpers-pure"; + +export enum FilterKey { + All = "all", + WithResults = "withResults", +} export enum SortKey { Name = "name", @@ -13,6 +19,7 @@ export enum SortKey { export type RepositoriesFilterSortState = { searchValue: string; + filterKey: FilterKey; sortKey: SortKey; }; @@ -22,20 +29,43 @@ export type RepositoriesFilterSortStateWithIds = RepositoriesFilterSortState & { export const defaultFilterSortState: RepositoriesFilterSortState = { searchValue: "", + filterKey: FilterKey.All, sortKey: SortKey.Name, }; export function matchesFilter( - repo: Pick, + item: FilterAndSortableResult, filterSortState: RepositoriesFilterSortState | undefined, ): boolean { if (!filterSortState) { return true; } - return repo.fullName - .toLowerCase() - .includes(filterSortState.searchValue.toLowerCase()); + return ( + matchesSearch(item.repository, filterSortState.searchValue) && + matchesFilterKey(item.resultCount, filterSortState.filterKey) + ); +} + +function matchesSearch( + repository: SortableRepository, + searchValue: string, +): boolean { + return repository.fullName.toLowerCase().includes(searchValue.toLowerCase()); +} + +function matchesFilterKey( + resultCount: number | undefined, + filterKey: FilterKey, +): boolean { + switch (filterKey) { + case FilterKey.All: + return true; + case FilterKey.WithResults: + return resultCount !== undefined && resultCount > 0; + default: + assertNever(filterKey); + } } type SortableRepository = Pick & @@ -71,17 +101,22 @@ export function compareRepository( }; } -type SortableResult = { +type FilterAndSortableResult = { + repository: SortableRepository; + resultCount?: number; +}; + +type FilterAndSortableResultWithIds = { repository: SortableRepository & Pick; resultCount?: number; }; export function compareWithResults( filterSortState: RepositoriesFilterSortState | undefined, -): (left: SortableResult, right: SortableResult) => number { +): (left: FilterAndSortableResult, right: FilterAndSortableResult) => number { const fallbackSort = compareRepository(filterSortState); - return (left: SortableResult, right: SortableResult) => { + return (left: FilterAndSortableResult, right: FilterAndSortableResult) => { // Highest to lowest if (filterSortState?.sortKey === SortKey.ResultsCount) { const resultCount = (right.resultCount ?? 0) - (left.resultCount ?? 0); @@ -95,7 +130,7 @@ export function compareWithResults( } export function filterAndSortRepositoriesWithResultsByName< - T extends SortableResult, + T extends FilterAndSortableResult, >( repositories: T[] | undefined, filterSortState: RepositoriesFilterSortState | undefined, @@ -105,11 +140,13 @@ export function filterAndSortRepositoriesWithResultsByName< } return repositories - .filter((repo) => matchesFilter(repo.repository, filterSortState)) + .filter((repo) => matchesFilter(repo, filterSortState)) .sort(compareWithResults(filterSortState)); } -export function filterAndSortRepositoriesWithResults( +export function filterAndSortRepositoriesWithResults< + T extends FilterAndSortableResultWithIds, +>( repositories: T[] | undefined, filterSortState: RepositoriesFilterSortStateWithIds | undefined, ): T[] | undefined { @@ -117,6 +154,7 @@ export function filterAndSortRepositoriesWithResults( return undefined; } + // If repository IDs are given, then ignore the search value and filter key if ( filterSortState?.repositoryIds && filterSortState.repositoryIds.length > 0 diff --git a/extensions/ql-vscode/src/stories/variant-analysis/RepositoriesFilter.stories.tsx b/extensions/ql-vscode/src/stories/variant-analysis/RepositoriesFilter.stories.tsx new file mode 100644 index 00000000000..8110fba144e --- /dev/null +++ b/extensions/ql-vscode/src/stories/variant-analysis/RepositoriesFilter.stories.tsx @@ -0,0 +1,25 @@ +import * as React from "react"; +import { useState } from "react"; + +import { ComponentMeta } from "@storybook/react"; + +import { RepositoriesFilter as RepositoriesFilterComponent } from "../../view/variant-analysis/RepositoriesFilter"; +import { FilterKey } from "../../pure/variant-analysis-filter-sort"; + +export default { + title: "Variant Analysis/Repositories Filter", + component: RepositoriesFilterComponent, + argTypes: { + value: { + control: { + disable: true, + }, + }, + }, +} as ComponentMeta; + +export const RepositoriesFilter = () => { + const [value, setValue] = useState(FilterKey.All); + + return ; +}; diff --git a/extensions/ql-vscode/src/view/variant-analysis/RepositoriesFilter.tsx b/extensions/ql-vscode/src/view/variant-analysis/RepositoriesFilter.tsx new file mode 100644 index 00000000000..b75c5bf3097 --- /dev/null +++ b/extensions/ql-vscode/src/view/variant-analysis/RepositoriesFilter.tsx @@ -0,0 +1,36 @@ +import * as React from "react"; +import { useCallback } from "react"; +import styled from "styled-components"; +import { VSCodeDropdown, VSCodeOption } from "@vscode/webview-ui-toolkit/react"; +import { Codicon } from "../common"; +import { FilterKey } from "../../pure/variant-analysis-filter-sort"; + +const Dropdown = styled(VSCodeDropdown)` + width: 100%; +`; + +type Props = { + value: FilterKey; + onChange: (value: FilterKey) => void; + + className?: string; +}; + +export const RepositoriesFilter = ({ value, onChange, className }: Props) => { + const handleInput = useCallback( + (e: InputEvent) => { + const target = e.target as HTMLSelectElement; + + onChange(target.value as FilterKey); + }, + [onChange], + ); + + return ( + + + All + With results + + ); +}; diff --git a/extensions/ql-vscode/src/view/variant-analysis/RepositoriesSearchSortRow.tsx b/extensions/ql-vscode/src/view/variant-analysis/RepositoriesSearchSortRow.tsx index 9d1b1fadfeb..229b6900d4e 100644 --- a/extensions/ql-vscode/src/view/variant-analysis/RepositoriesSearchSortRow.tsx +++ b/extensions/ql-vscode/src/view/variant-analysis/RepositoriesSearchSortRow.tsx @@ -2,11 +2,13 @@ import * as React from "react"; import { Dispatch, SetStateAction, useCallback } from "react"; import styled from "styled-components"; import { + FilterKey, RepositoriesFilterSortState, SortKey, } from "../../pure/variant-analysis-filter-sort"; import { RepositoriesSearch } from "./RepositoriesSearch"; import { RepositoriesSort } from "./RepositoriesSort"; +import { RepositoriesFilter } from "./RepositoriesFilter"; type Props = { value: RepositoriesFilterSortState; @@ -25,6 +27,10 @@ const RepositoriesSearchColumn = styled(RepositoriesSearch)` flex: 3; `; +const RepositoriesFilterColumn = styled(RepositoriesFilter)` + flex: 1; +`; + const RepositoriesSortColumn = styled(RepositoriesSort)` flex: 1; `; @@ -40,6 +46,16 @@ export const RepositoriesSearchSortRow = ({ value, onChange }: Props) => { [onChange], ); + const handleFilterKeyChange = useCallback( + (filterKey: FilterKey) => { + onChange((oldValue) => ({ + ...oldValue, + filterKey, + })); + }, + [onChange], + ); + const handleSortKeyChange = useCallback( (sortKey: SortKey) => { onChange((oldValue) => ({ @@ -56,6 +72,10 @@ export const RepositoriesSearchSortRow = ({ value, onChange }: Props) => { value={value.searchValue} onChange={handleSearchValueChange} /> + { const repositories = useMemo(() => { return skippedRepositoryGroup.repositories - ?.filter((repo) => { - return matchesFilter(repo, filterSortState); + ?.filter((repository) => { + return matchesFilter({ repository }, filterSortState); }) ?.sort(compareRepository(filterSortState)); }, [filterSortState, skippedRepositoryGroup.repositories]); diff --git a/extensions/ql-vscode/test/unit-tests/variant-analysis-filter-sort.test.ts b/extensions/ql-vscode/test/unit-tests/variant-analysis-filter-sort.test.ts index 601ddaaeb61..90319ee6568 100644 --- a/extensions/ql-vscode/test/unit-tests/variant-analysis-filter-sort.test.ts +++ b/extensions/ql-vscode/test/unit-tests/variant-analysis-filter-sort.test.ts @@ -4,6 +4,7 @@ import { defaultFilterSortState, filterAndSortRepositoriesWithResults, filterAndSortRepositoriesWithResultsByName, + FilterKey, matchesFilter, SortKey, } from "../../src/pure/variant-analysis-filter-sort"; @@ -13,32 +14,93 @@ describe(matchesFilter.name, () => { fullName: "github/codeql", }; - const testCases = [ - { searchValue: "", matches: true }, - { searchValue: "github/codeql", matches: true }, - { searchValue: "github", matches: true }, - { searchValue: "git", matches: true }, - { searchValue: "codeql", matches: true }, - { searchValue: "code", matches: true }, - { searchValue: "ql", matches: true }, - { searchValue: "/", matches: true }, - { searchValue: "gothub/codeql", matches: false }, - { searchValue: "hello", matches: false }, - { searchValue: "cod*ql", matches: false }, - { searchValue: "cod?ql", matches: false }, - ]; + describe("searchValue", () => { + const testCases = [ + { searchValue: "", matches: true }, + { searchValue: "github/codeql", matches: true }, + { searchValue: "github", matches: true }, + { searchValue: "git", matches: true }, + { searchValue: "codeql", matches: true }, + { searchValue: "code", matches: true }, + { searchValue: "ql", matches: true }, + { searchValue: "/", matches: true }, + { searchValue: "gothub/codeql", matches: false }, + { searchValue: "hello", matches: false }, + { searchValue: "cod*ql", matches: false }, + { searchValue: "cod?ql", matches: false }, + ]; + + test.each(testCases)( + "returns $matches if searching for $searchValue", + ({ searchValue, matches }) => { + expect( + matchesFilter( + { repository }, + { + ...defaultFilterSortState, + searchValue, + }, + ), + ).toBe(matches); + }, + ); + }); - test.each(testCases)( - "returns $matches if searching for $searchValue", - ({ searchValue, matches }) => { + describe("filterKey", () => { + it("returns true if filterKey is all and resultCount is positive", () => { expect( - matchesFilter(repository, { - ...defaultFilterSortState, - searchValue, - }), - ).toBe(matches); - }, - ); + matchesFilter( + { repository, resultCount: 1 }, + { ...defaultFilterSortState, filterKey: FilterKey.All }, + ), + ).toBe(true); + }); + + it("returns true if filterKey is all and resultCount is zero", () => { + expect( + matchesFilter( + { repository, resultCount: 0 }, + { ...defaultFilterSortState, filterKey: FilterKey.All }, + ), + ).toBe(true); + }); + + it("returns true if filterKey is all and resultCount is undefined", () => { + expect( + matchesFilter( + { repository }, + { ...defaultFilterSortState, filterKey: FilterKey.All }, + ), + ).toBe(true); + }); + + it("returns true if filterKey is withResults and resultCount is positive", () => { + expect( + matchesFilter( + { repository, resultCount: 1 }, + { ...defaultFilterSortState, filterKey: FilterKey.WithResults }, + ), + ).toBe(true); + }); + + it("returns false if filterKey is withResults and resultCount is zero", () => { + expect( + matchesFilter( + { repository, resultCount: 0 }, + { ...defaultFilterSortState, filterKey: FilterKey.WithResults }, + ), + ).toBe(false); + }); + + it("returns false if filterKey is withResults and resultCount is undefined", () => { + expect( + matchesFilter( + { repository }, + { ...defaultFilterSortState, filterKey: FilterKey.WithResults }, + ), + ).toBe(false); + }); + }); }); describe(compareRepository.name, () => { @@ -349,7 +411,7 @@ describe(filterAndSortRepositoriesWithResultsByName.name, () => { }, ]; - describe("when sort key is given without filter", () => { + describe("when sort key is given without search or filter", () => { it("returns the correct results", () => { expect( filterAndSortRepositoriesWithResultsByName(repositories, { @@ -365,7 +427,7 @@ describe(filterAndSortRepositoriesWithResultsByName.name, () => { }); }); - describe("when sort key and search filter are given", () => { + describe("when sort key and search are given without filter", () => { it("returns the correct results", () => { expect( filterAndSortRepositoriesWithResultsByName(repositories, { @@ -376,6 +438,30 @@ describe(filterAndSortRepositoriesWithResultsByName.name, () => { ).toEqual([repositories[2], repositories[0]]); }); }); + + describe("when sort key and filter withResults are given without search", () => { + it("returns the correct results", () => { + expect( + filterAndSortRepositoriesWithResultsByName(repositories, { + ...defaultFilterSortState, + sortKey: SortKey.ResultsCount, + filterKey: FilterKey.WithResults, + }), + ).toEqual([repositories[3], repositories[2], repositories[0]]); + }); + }); + + describe("when sort key, search, and filter withResults are given", () => { + it("returns the correct results", () => { + expect( + filterAndSortRepositoriesWithResultsByName(repositories, { + sortKey: SortKey.ResultsCount, + filterKey: FilterKey.WithResults, + searchValue: "r", + }), + ).toEqual([repositories[3]]); + }); + }); }); describe(filterAndSortRepositoriesWithResults.name, () => { @@ -410,7 +496,7 @@ describe(filterAndSortRepositoriesWithResults.name, () => { }, ]; - describe("when sort key is given without filter", () => { + describe("when sort key is given", () => { it("returns the correct results", () => { expect( filterAndSortRepositoriesWithResults(repositories, { @@ -426,7 +512,7 @@ describe(filterAndSortRepositoriesWithResults.name, () => { }); }); - describe("when sort key and search filter are given", () => { + describe("when sort key and search are given", () => { it("returns the correct results", () => { expect( filterAndSortRepositoriesWithResults(repositories, { @@ -438,12 +524,49 @@ describe(filterAndSortRepositoriesWithResults.name, () => { }); }); - describe("when sort key, search filter, and repository ids are given", () => { + describe("when sort key and filter withResults are given", () => { + it("returns the correct results", () => { + expect( + filterAndSortRepositoriesWithResults(repositories, { + ...defaultFilterSortState, + sortKey: SortKey.ResultsCount, + filterKey: FilterKey.WithResults, + }), + ).toEqual([repositories[3], repositories[2], repositories[0]]); + }); + }); + + describe("when sort key and filter withResults are given", () => { + it("returns the correct results", () => { + expect( + filterAndSortRepositoriesWithResults(repositories, { + ...defaultFilterSortState, + sortKey: SortKey.ResultsCount, + filterKey: FilterKey.WithResults, + }), + ).toEqual([repositories[3], repositories[2], repositories[0]]); + }); + }); + + describe("when sort key, search, and filter withResults are given", () => { it("returns the correct results", () => { expect( filterAndSortRepositoriesWithResults(repositories, { ...defaultFilterSortState, sortKey: SortKey.ResultsCount, + filterKey: FilterKey.WithResults, + searchValue: "r", + }), + ).toEqual([repositories[3]]); + }); + }); + + describe("when sort key, search, filter withResults, and repository ids are given", () => { + it("returns the correct results", () => { + expect( + filterAndSortRepositoriesWithResults(repositories, { + sortKey: SortKey.ResultsCount, + filterKey: FilterKey.WithResults, searchValue: "la", repositoryIds: [ repositories[1].repository.id,