diff --git a/src/linter/rules/deprecated-functions.ts b/src/linter/rules/deprecated-functions.ts index 35a1c1a06ff..744cc78008b 100644 --- a/src/linter/rules/deprecated-functions.ts +++ b/src/linter/rules/deprecated-functions.ts @@ -1,36 +1,186 @@ -import { VertexType } from '../../dataflow/graph/vertex'; -import { Dataflow } from '../../dataflow/graph/df-helper'; +import type { Range } from 'semver'; +import type { BrandedIdentifier } from '../../dataflow/environments/identifier'; import { Identifier } from '../../dataflow/environments/identifier'; -import { Q } from '../../search/flowr-search-builder'; +import type { DataflowGraph } from '../../dataflow/graph/graph'; +import { FunctionArgument } from '../../dataflow/graph/graph'; +import { FunctionCallVertex, VertexType } from '../../dataflow/graph/vertex'; +import { EmptyArgument } from '../../r-bridge/lang-4.x/ast/model/nodes/r-function-call'; import { Enrichment, enrichmentContent } from '../../search/search-executor/search-enrichers'; +import { isNotUndefined } from '../../util/assert'; +import type { MergeableRecord } from '../../util/objects'; import { SourceLocation } from '../../util/range'; -import { LintingRuleCertainty, LintingResultCertainty, type LintingRule } from '../linter-format'; +import type { LintingResult, LintingRule } from '../linter-format'; +import { LintingPrettyPrintContext, LintingResultCertainty, LintingRuleCertainty } from '../linter-format'; import { LintingRuleTag } from '../linter-tags'; -import { type FunctionsMetadata, type FunctionsResult, type FunctionsToDetectConfig, functionFinderUtil } from './function-finder-util'; +import { RRange } from '../../util/r-version'; +import { Q } from '../../search/flowr-search-builder'; +import type { RNode } from '../../r-bridge/lang-4.x/ast/model/model'; +import type { AstIdMap, ParentInformation } from '../../r-bridge/lang-4.x/ast/model/processing/decorate'; +import { Dataflow } from '../../dataflow/graph/df-helper'; +import type { ReadonlyFlowrAnalysisProvider } from '../../project/flowr-analyzer'; +import { hasArgumentValue } from './function-finder-util'; +import { Ternary } from '../../util/logic'; +import type { KnownParser } from '../../r-bridge/parser'; + +/** + * Information about an argument of a function that should be flagged as deprecated if it is called with this argument + * + * Used in {@link DeprecatedFunctionInformation} to mark a function argument as deprecate under certain conditions + */ +interface DeprecatedArgumentInformation { + /** Index of the argument */ + readonly argIdx?: number, + /** Name of the argument */ + readonly argName?: string + /** Only mark this argument as deprecated, if a specific value was provided */ + readonly ifValue?: RegExp | string + /** Suggested replacement for this argument */ + readonly replacedBy?: string + /** The version since this argument is deprecated */ + readonly sinceVersion?: Range + /** The state of deprecation {@link DeprecationState}, i.e. is the argument completely removed, or are there better alternatives */ + readonly state?: DeprecationState +} + +/** + * Information about a deprecated function + * + * Used in {@link DeprecatedFunctionsConfig.conditionally} to mark a function as deprecate under certain conditions + */ +interface DeprecatedFunctionInformation { + /** + * Mark specific arguments as deprecated + * If only whenArgs is provided, and not sinceVersion, the function is only marked as deprecated, if the argument is provided. + */ + readonly whenArgs?: DeprecatedArgumentInformation[] + /** Suggested replacement for this function */ + readonly replacedBy?: string + /** The version since this function is deprecated, if version is provided the entire function will be marked as deprecated, if the version range matches */ + readonly sinceVersion?: Range + /** Lifecycle State {@link DeprecationState}, i.e. is the function completely removed, or are there better alternatives */ + readonly state?: DeprecationState + /** The package this function comes from */ + readonly package: string +} + +/** + * Result of the {@link DEPRECATED_FUNCTIONS} linting rule + * See also the specializations {@link DeprecatedFunctionResult} and {@link DeprecatedArgumentResult} + */ +export interface DeprecatedFunctionResultBase extends LintingResult { + /** The function affected by the deprecation */ + readonly function: Identifier + /** The suggest replacement for the deprecated argument or function */ + readonly replacedBy?: string + /** Since which package version this argument or function is deprecated */ + readonly sinceVersion?: Range + /** Lifecycle State {@link DeprecationState} */ + readonly state?: DeprecationState +} + +/** + * Returned by the {@link DEPRECATED_FUNCTIONS} linting rule, when a deprecated function is detected. + * Provided for convince to differentiate between {@link DeprecatedArgumentResult} and {@link DeprecatedFunctionResult} + */ +export interface DeprecatedFunctionResult extends DeprecatedFunctionResultBase { + readonly type: 'deprecated-function' +} + +/** + * Returned by the {@link DEPRECATED_FUNCTIONS} linting rule, when a deprecated argument is detected. + * Provided for convince to differentiate between {@link DeprecatedArgumentResult} and {@link DeprecatedFunctionResult} + */ +export interface DeprecatedArgumentResult extends DeprecatedFunctionResultBase { + readonly type: 'deprecated-argument' + /** The name or index of the deprecated argument */ + readonly arg: string | number +} + +export type DeprecatedFunctionRuleResult = DeprecatedFunctionResult | DeprecatedArgumentResult; + +export enum DeprecationState { + /** A better alternative is available, but the function is kept (softer alternative to deprecated) {@link https://lifecycle.r-lib.org/articles/stages.html#superseded} */ + Superseeded = 'superseeded', + /** A better alternative is available, and the function is marked for removal {@link https://lifecycle.r-lib.org/articles/stages.html#deprecated} */ + Deprecated = 'deprecated', + /** No longer works and is removed and replaced by another function {@link https://www.rdocumentation.org/packages/base/versions/3.6.2/topics/Defunct} */ + Defunct = 'defunct' +} + +export interface DeprecatedFunctionsConfig extends MergeableRecord { + /** Functions to always mark as deprecated */ + always: Identifier[] + /** Functions to mark as deprecated for specific argument, argument value or version */ + conditionally: Record +} + +interface PotentialFunction { + node: RNode; + target: Identifier; + sourceLocation: SourceLocation +} + +interface Metadata extends MergeableRecord { + sigdb: number, + hardcoded: number +} + +const AlwaysDeprecated = [ + 'all_equal', 'arrange_all', 'distinct_all', 'filter_all', 'group_by_all', 'summarise_all', 'mutate_all', 'select_all', 'vars', 'all_vars', 'id', 'failwith', 'select_vars', 'rename_vars', 'select_var', 'current_vars', 'bench_tbls', 'compare_tbls', 'compare_tbls2', 'eval_tbls', 'eval_tbls2', 'location', 'changes', 'combine', 'do', 'funs', 'add_count_', 'add_tally_', 'arrange1_', 'count_', 'distinct_', 'do_', 'filter_', 'funs_', 'group_by_', 'group_indices_', 'mutate_', 'tally_', 'transmute_', 'rename_', 'rename_vars_', 'select_', 'select_vars_', 'slice_', 'summarise_', 'summarize_', 'summarise_each', 'src_local', 'tbl_df', 'add_rownames', 'group_nest', 'group_split', 'with_groups', 'nest_by', 'progress_estimated', 'recode', 'sample_n', 'top_n', 'transmute', 'fct_explicit_na', 'aes_', 'aes_auto', 'annotation_logticks', 'is.Coord', 'coord_flip', 'coord_map', 'is.facet', 'fortify', 'is.ggproto', 'guide_train', 'is.ggplot', 'qplot', 'is.theme', 'gg_dep', 'liply', 'isplit2', 'list_along', 'cross', 'invoke', 'at_depth', 'prepend', 'rerun', 'splice', '`%@%`', 'rbernoulli', 'rdunif', 'when', 'update_list', 'map_raw', 'accumulate', 'reduce_right', 'flatten', 'map_dfr', 'as_vector', 'transpose', 'melt_delim', 'melt_fwf', 'melt_table', 'read_table2', 'str_interp', 'as_tibble', 'data_frame', 'tibble_', 'data_frame_', 'lst_', 'as_data_frame', 'as.tibble', 'frame_data', 'trunc_mat', 'is.tibble', 'tidy_names', 'set_tidy_names', 'repair_names', 'extract_numeric', 'complete_', 'drop_na_', 'expand_', 'crossing_', 'nesting_', 'extract_', 'fill_', 'gather_', 'nest_', 'separate_rows_', 'separate_', 'spread_', 'unite_', 'unnest_', 'extract', 'gather', 'nest_legacy', 'separate_rows', 'separate', 'spread' +]; + +const ConditionallyDeprecated = { + 'geom_violin': { package: 'ggplot2', whenArgs: [{ argName: 'draw_quantiles', state: DeprecationState.Deprecated, replacedBy: 'quantile.linetype', sinceVersion: RRange.parse('>= 4.0.0') }] }, +} satisfies Record; export const DEPRECATED_FUNCTIONS = { // unlike functionFinderUtil.createSearch(config.fns), this does not pre-filter to the hardcoded list: the // sigdb-driven pass below needs every resolved call, so the `fns` filtering happens in processSearchResult instead - createSearch: (_config: FunctionsToDetectConfig) => Q.all().filter(VertexType.FunctionCall).with(Enrichment.CallTargets, { + createSearch: (_config) => Q.all().filter(VertexType.FunctionCall).with(Enrichment.CallTargets, { onlyBuiltin: true, - qualifyNames: false // we don't use qualified names for this rule yet + qualifyNames: true // we don't use qualified names for this rule yet }), processSearchResult: async(elements, config, data) => { - const matchesConfiguredFns = Identifier.regex(...config.fns); - const hardcoded = await functionFinderUtil.processSearchResult(elements, config, data, es => - es.filter(e => enrichmentContent(e, Enrichment.CallTargets)?.targets - .some(t => typeof t === 'string' && matchesConfiguredFns.test(t)))); + const matchesConfiguredFns = Identifier.regex(...config.always); + const graph = (await data.dataflow()).graph; + const idMap = (await data.normalize()).idMap; + + // 1. Collect all function call targets from detected function calls + const detectedFunctions = elements.getElements().flatMap(e => { + return enrichmentContent(e, Enrichment.CallTargets).targets.map(target => { + const sourceLocation = SourceLocation.fromNode(e.node); + if(sourceLocation !== undefined) { + return { + node: e.node, target: target as string, sourceLocation + }; + } + }); + }).filter(p => isNotUndefined(p)); + // 2. Uses hardcoded information about deprecated arguments and deprecated functions + const results: DeprecatedFunctionRuleResult[] = (await Promise.all(detectedFunctions.map(async candidate => { + const name = candidate.target.includes('::') ? Identifier.getName(Identifier.parse(candidate.target)) : candidate.target; + const info = config.conditionally[name]; + if(isNotUndefined(info)) { + // Check functions from DeprecatedFunctionsConfig.conditionally + return await deprecateFunctionConditionally(candidate, graph, idMap, data, info); + } else { + // Check functions from DeprecatedFunctionsConfig.always + return deprecateFunctionAlways(candidate, matchesConfiguredFns); + } + }))).filter(p => isNotUndefined(p)).flat(); + + + // 3. If available, use sigdb to flag deprecated functions const deps = data.inspectContext().deps; if(deps.signatureSources().length === 0) { - return hardcoded; + return { results, '.meta': { hardcoded: results.length, sigdb: 0 } }; } // sigdb-driven detection: flag any resolved call whose signature-database entry marks it deprecated, // even when it is not part of the hardcoded `fns` list above - const graph = (await data.dataflow()).graph; - const alreadyFlagged = new Set(hardcoded.results.map(r => r.involvedId)); - const sigdbFlagged: FunctionsResult[] = []; + const alreadyFlagged = new Set(results.map(r => r.involvedId)); + const sigdbFlagged: DeprecatedFunctionResult[] = []; for(const element of elements.getElements()) { const id = element.node.info.id; if(alreadyFlagged.has(id)) { @@ -50,6 +200,7 @@ export const DEPRECATED_FUNCTIONS = { } alreadyFlagged.add(id); sigdbFlagged.push({ + type: 'deprecated-function', certainty: LintingResultCertainty.Certain, involvedId: id, function: Identifier.toString(qualified), @@ -58,23 +209,141 @@ export const DEPRECATED_FUNCTIONS = { } return { - results: [...hardcoded.results, ...sigdbFlagged], - '.meta': { - totalCalls: hardcoded['.meta'].totalCalls + sigdbFlagged.length, - totalFunctionDefinitions: hardcoded['.meta'].totalFunctionDefinitions + sigdbFlagged.length - } + results: results.concat(sigdbFlagged), + '.meta': { hardcoded: results.length, sigdb: sigdbFlagged.length } }; }, - prettyPrint: functionFinderUtil.prettyPrint('deprecated'), - info: { + prettyPrint: { + [LintingPrettyPrintContext.Query]: (result: DeprecatedFunctionRuleResult) => `${result.type === 'deprecated-argument' ? `Argument \`${result.arg}\` of ` : ''}Function \`${Identifier.toString(result.function)}\` at ${SourceLocation.format(result.loc)}`, + [LintingPrettyPrintContext.Full]: (result: DeprecatedFunctionRuleResult) => { + const str: string[] = []; + if(result.type === 'deprecated-argument') { + const argStr = typeof result.arg === 'number' ? `at position \`${result.arg}\`` : result.arg; + str.push(`Argument \`${argStr}\` of`); + } + str.push(`Function \`${Identifier.toString(result.function)}\` is ${result.state ?? 'deprecated'}`); + if(result.sinceVersion) { + str.push(`since version ${result.sinceVersion.format()}`); + } + if(result.replacedBy) { + str.push(`and is replaced by \`${result.replacedBy}\``); + } + return str.join(' '); + } + }, + info: { name: 'Deprecated Functions', tags: [LintingRuleTag.Deprecated, LintingRuleTag.Smell, LintingRuleTag.Usability, LintingRuleTag.Reproducibility], - // the hardcoded `fns` list ensures every reported hit is real, but the list is pre-crawled and hence + // the hardcoded `always` and `conditionally` list ensures every reported hit is real, but the list is pre-crawled and hence // incomplete; the signature-database pass above adds recall for whichever packages are resolved certainty: LintingRuleCertainty.BestEffort, description: 'Marks deprecated functions that should not be used anymore.', defaultConfig: { - fns: ['all_equal', 'arrange_all', 'distinct_all', 'filter_all', 'group_by_all', 'summarise_all', 'mutate_all', 'select_all', 'vars', 'all_vars', 'id', 'failwith', 'select_vars', 'rename_vars', 'select_var', 'current_vars', 'bench_tbls', 'compare_tbls', 'compare_tbls2', 'eval_tbls', 'eval_tbls2', 'location', 'changes', 'combine', 'do', 'funs', 'add_count_', 'add_tally_', 'arrange_', 'count_', 'distinct_', 'do_', 'filter_', 'funs_', 'group_by_', 'group_indices_', 'mutate_', 'tally_', 'transmute_', 'rename_', 'rename_vars_', 'select_', 'select_vars_', 'slice_', 'summarise_', 'summarize_', 'summarise_each', 'src_local', 'tbl_df', 'add_rownames', 'group_nest', 'group_split', 'with_groups', 'nest_by', 'progress_estimated', 'recode', 'sample_n', 'top_n', 'transmute', 'fct_explicit_na', 'aes_', 'aes_auto', 'annotation_logticks', 'is.Coord', 'coord_flip', 'coord_map', 'is.facet', 'fortify', 'is.ggproto', 'guide_train', 'is.ggplot', 'qplot', 'is.theme', 'gg_dep', 'liply', 'isplit2', 'list_along', 'cross', 'invoke', 'at_depth', 'prepend', 'rerun', 'splice', '`%@%`', 'rbernoulli', 'rdunif', 'when', 'update_list', 'map_raw', 'accumulate', 'reduce_right', 'flatten', 'map_dfr', 'as_vector', 'transpose', 'melt_delim', 'melt_fwf', 'melt_table', 'read_table2', 'str_interp', 'as_tibble', 'data_frame', 'tibble_', 'data_frame_', 'lst_', 'as_data_frame', 'as.tibble', 'frame_data', 'trunc_mat', 'is.tibble', 'tidy_names', 'set_tidy_names', 'repair_names', 'extract_numeric', 'complete_', 'drop_na_', 'expand_', 'crossing_', 'nesting_', 'extract_', 'fill_', 'gather_', 'nest_', 'separate_rows_', 'separate_', 'spread_', 'unite_', 'unnest_', 'extract', 'gather', 'nest_legacy', 'separate_rows', 'separate', 'spread'] + always: AlwaysDeprecated, + conditionally: ConditionallyDeprecated } } -} as const satisfies LintingRule; +} as const satisfies LintingRule; + +/** + * This function is applied to function candidates that have an entry in the {@link DeprecatedFunctionsConfig.conditionally} map. + */ +async function deprecateFunctionConditionally(candidate: PotentialFunction, dataflow: DataflowGraph, idMap: AstIdMap, analyzer: ReadonlyFlowrAnalysisProvider, info: DeprecatedFunctionInformation): Promise { + const results: DeprecatedFunctionRuleResult[] = []; + const result = await analyzer.query([{ + type: 'guess-dep-versions', + packages: [info.package] + }]); + const derrivedRangeRaw = result['guess-dep-versions'].dependencies.find(d => d.package === info.package)?.range; + const derrivedRange = derrivedRangeRaw !== undefined ? RRange.parse(derrivedRangeRaw) : undefined; + + // Deprecated Argument: If `whenArgs` is provided, only mark deprecated arguments + if(info.whenArgs) { + const vertex = dataflow.getVertex(candidate.node.info.id); + if(vertex === undefined || !FunctionCallVertex.is(vertex)) { + return results; + } + + for(const deprecatedArgInfo of info.whenArgs) { + // Check if function call has deprecated argument + const arg = vertex.args.find((arg, idx) => + FunctionArgument.isNamed(arg) && arg.name === deprecatedArgInfo.argName || + FunctionArgument.isPositional(arg) && idx === deprecatedArgInfo.argIdx + ); + const argNode = arg === undefined || arg === EmptyArgument ? undefined : idMap.get(arg.nodeId); + if(argNode === undefined) { + continue; + } + + // If `sinceVersion` is set, check package version before marking argument as deprecated + let certainty = LintingResultCertainty.Certain; + if(deprecatedArgInfo.sinceVersion) { + if(derrivedRange == undefined) { + certainty = LintingResultCertainty.Uncertain; + } else if(!deprecatedArgInfo.sinceVersion.intersects(derrivedRange)) { + continue; + } + } + + // If `ifValue` is set, check argument value before marking argument as deprecate + if(deprecatedArgInfo.ifValue) { + const hasArg = hasArgumentValue(deprecatedArgInfo.ifValue, vertex, analyzer, dataflow, true, deprecatedArgInfo.argName, deprecatedArgInfo.argIdx); + if(hasArg === Ternary.Never) { + continue; + } else if(hasArg === Ternary.Maybe) { + certainty = LintingResultCertainty.Uncertain; + } + } + + // If all checks passed, mark as deprecated + results.push({ + type: 'deprecated-argument', + certainty: certainty, + involvedId: argNode.info.id, + function: candidate.target, + arg: (deprecatedArgInfo.argName ?? deprecatedArgInfo.argIdx) as string | number, + state: deprecatedArgInfo.state, + replacedBy: deprecatedArgInfo.replacedBy, + sinceVersion: deprecatedArgInfo.sinceVersion, + loc: SourceLocation.fromNode(argNode) ?? candidate.sourceLocation + } satisfies DeprecatedArgumentResult); + } + } + + // Deprecated Function: If `sinceVersion` is set, check package version before marking as deprecated + if(info.sinceVersion) { + const isDeprecatedVersion = derrivedRange ? info.sinceVersion.intersects(derrivedRange) : undefined; + if(isDeprecatedVersion === true || isDeprecatedVersion === undefined) { + results.push({ + type: 'deprecated-function', + certainty: isDeprecatedVersion === undefined ? LintingResultCertainty.Uncertain : LintingResultCertainty.Certain, + involvedId: candidate.node.info.id, + loc: candidate.sourceLocation, + function: candidate.target, + state: info.state, + replacedBy: info.replacedBy, + sinceVersion: info.sinceVersion + } satisfies DeprecatedFunctionResult); + } + } + + return results; +} + + +/** + * This function is applied to function candidates that have an entry in the {@link DeprecatedFunctionsConfig.always} map. + */ +function deprecateFunctionAlways(candidate: PotentialFunction, matchesConfiguredFns: RegExp): DeprecatedFunctionResult | undefined { + if(!matchesConfiguredFns.test(Identifier.getName(candidate.target))) { + return undefined; + } + + return { + type: 'deprecated-function', + certainty: LintingResultCertainty.Certain, + involvedId: candidate.node.info.id, + loc: candidate.sourceLocation, + function: candidate.target, + } satisfies DeprecatedFunctionResult; +} diff --git a/src/linter/rules/function-finder-util.ts b/src/linter/rules/function-finder-util.ts index 97dc7a21dad..8059a44b4af 100644 --- a/src/linter/rules/function-finder-util.ts +++ b/src/linter/rules/function-finder-util.ts @@ -8,6 +8,7 @@ import type { ParentInformation } from '../../r-bridge/lang-4.x/ast/model/proces import type { MergeableRecord } from '../../util/objects'; import { isNotUndefined } from '../../util/assert'; import { getArgumentStringValue } from '../../dataflow/eval/resolve/resolve-argument'; +import type { DataflowGraphVertexFunctionCall } from '../../dataflow/graph/vertex'; import { FunctionCallVertex, VertexType } from '../../dataflow/graph/vertex'; import type { FunctionInfo } from '../../queries/catalog/dependencies-query/function-info/function-info'; import { Unknown } from '../../queries/catalog/dependencies-query/dependencies-query-format'; @@ -16,6 +17,7 @@ import { Identifier } from '../../dataflow/environments/identifier'; import { Ternary } from '../../util/logic'; import type { ReadonlyFlowrAnalysisProvider } from '../../project/flowr-analyzer'; import type { AsyncOrSync } from 'ts-essentials'; +import type { DataflowGraph } from '../../dataflow/graph/graph'; import { Dataflow } from '../../dataflow/graph/df-helper'; export interface FunctionsResult extends LintingResult { @@ -105,35 +107,55 @@ export const functionFinderUtil = { ): Promise { const dataflow = await analyzer.dataflow(); const identifier = Dataflow.qualify(element.node.info.id, dataflow.graph, true) ?? (element.node.lexeme !== undefined ? Identifier.parse(element.node.lexeme) : undefined); + /* if we have no additional info, we assume they always access the network */ - if(identifier === undefined) { + if(identifier === undefined || requireValue === undefined) { return Ternary.Always; } + // we allow our function pool to contain non-namespaced functions const info = pool.get(Identifier.toString(identifier)) ?? pool.get(Identifier.getName(identifier)); if(info === undefined) { return Ternary.Always; } + const vert = dataflow.graph.getVertex(element.node.info.id); - if(FunctionCallVertex.is(vert)){ - const args = getArgumentStringValue( - analyzer.flowrConfig.solver.variables, - dataflow.graph, - vert, - info.argIdx, - info.argName, - info.resolveValue, - analyzer.inspectContext()); - // we obtain all values, at least one of them has to trigger for the request - const argValues: string[] = args ? args.values().flatMap(s => Array.from(s)).filter(isNotUndefined).toArray() : []; - if(argValues.length === 0){ - return Ternary.Maybe; - } else if(argValues.some(v => requireValue instanceof RegExp ? requireValue.test(v) : v === requireValue)){ - return Ternary.Always; - } else if(argValues.some(v => v === Unknown)) { - return Ternary.Maybe; - } + if(FunctionCallVertex.is(vert)) { + return hasArgumentValue(requireValue, vert, analyzer, dataflow.graph, info.resolveValue, info.argName, info.argIdx); } + return Ternary.Never; } }; + +/** + * Test if a function call has an argument with a specific value + */ +export function hasArgumentValue( + test: RegExp | string, + fnVertex: DataflowGraphVertexFunctionCall, + analyzer: ReadonlyFlowrAnalysisProvider, + dataflow: DataflowGraph, + resolveValue: boolean | 'library' | undefined, + argName?: string, + argIdx?: number | 'unnamed'): Ternary { + const args = getArgumentStringValue( + analyzer.flowrConfig.solver.variables, + dataflow, + fnVertex, + argIdx, + argName, + resolveValue, + analyzer.inspectContext()); + // we obtain all values, at least one of them has to trigger for the request + const argValues: string[] = args ? args.values().flatMap(s => Array.from(s)).filter(isNotUndefined).toArray() : []; + if(argValues.length === 0){ + return Ternary.Maybe; + } else if(argValues.some(v => test instanceof RegExp ? test.test(v) : v === test)){ + return Ternary.Always; + } else if(argValues.some(v => v === Unknown)) { + return Ternary.Maybe; + } + + return Ternary.Never; +} diff --git a/src/linter/rules/naming-convention.ts b/src/linter/rules/naming-convention.ts index 9b108326709..d8a3d6730a9 100644 --- a/src/linter/rules/naming-convention.ts +++ b/src/linter/rules/naming-convention.ts @@ -118,7 +118,7 @@ export function getMostUsedCasing(symbols: { detectedCasing: CasingConvention }[ map.set(symbol.detectedCasing, o + 1); } - // Return element with most occurances + // Return element with most occurrences return [...map].reduce((p, c) => p[1] > c[1] ? p : c)[0]; } diff --git a/src/linter/rules/network-functions.ts b/src/linter/rules/network-functions.ts index 446d8b8d816..5a9b28226cc 100644 --- a/src/linter/rules/network-functions.ts +++ b/src/linter/rules/network-functions.ts @@ -1,7 +1,7 @@ import { LintingResultCertainty, type LintingRule, LintingRuleCertainty } from '../linter-format'; -import { functionFinderUtil, type FunctionsMetadata, type FunctionsResult } from './function-finder-util'; +import type { FunctionsMetadata, FunctionsResult } from './function-finder-util'; +import { functionFinderUtil } from './function-finder-util'; import { LintingRuleTag } from '../linter-tags'; -import type { MergeableRecord } from '../../util/objects'; import { ReadFunctions } from '../../queries/catalog/dependencies-query/function-info/read-functions'; import type { FlowrSearchElement } from '../../search/flowr-search'; import type { ParentInformation } from '../../r-bridge/lang-4.x/ast/model/processing/decorate'; @@ -11,6 +11,7 @@ import { WriteFunctions } from '../../queries/catalog/dependencies-query/functio import type { FunctionInfo } from '../../queries/catalog/dependencies-query/function-info/function-info'; import { Identifier } from '../../dataflow/environments/identifier'; import { Dataflow } from '../../dataflow/graph/df-helper'; +import type { MergeableRecord } from '../../util/objects'; export interface NetworkFunctionsConfig extends MergeableRecord { /** @@ -18,6 +19,7 @@ export interface NetworkFunctionsConfig extends MergeableRecord { */ fns: readonly (Identifier | NetworkFunction)[] } + export interface NetworkFunction extends MergeableRecord{ /** * The name of the network function to find. diff --git a/test/functionality/_helper/linter.ts b/test/functionality/_helper/linter.ts index 95f7cff1b99..6556b32f456 100644 --- a/test/functionality/_helper/linter.ts +++ b/test/functionality/_helper/linter.ts @@ -64,6 +64,8 @@ export function controlledSigDb(pkgOrPkgs: string | Record = T extends unknown ? Omit : never; + /** * Asserts correct linting results while ignoring each linting result's {@link LintingRuleResult.involvedId}. */ @@ -72,7 +74,7 @@ export function assertLinter( parser: KnownParser, code: string, ruleName: Name, - expected: Omit, 'involvedId'>[] | ((df: DataflowInformation, ast: NormalizedAst) => Omit, 'involvedId'>[]), + expected: DistributiveOmit, 'involvedId'>[] | ((df: DataflowInformation, ast: NormalizedAst) => Omit, 'involvedId'>[]), expectedMetadata?: LintingRuleMetadata, lintingRuleConfig?: DeepPartial> & LinterTestSetup ) { diff --git a/test/functionality/linter/lint-deprecated-functions.test.ts b/test/functionality/linter/lint-deprecated-functions.test.ts index 2e69a3cb3d5..f92d6caf556 100644 --- a/test/functionality/linter/lint-deprecated-functions.test.ts +++ b/test/functionality/linter/lint-deprecated-functions.test.ts @@ -2,9 +2,17 @@ import { describe } from 'vitest'; import { withTreeSitter } from '../_helper/shell'; import { assertLinter, controlledSigDb } from '../_helper/linter'; import { LintingResultCertainty } from '../../../src/linter/linter-format'; +import { DeprecationState } from '../../../src/linter/rules/deprecated-functions'; import type { PackageSignatureSource } from '../../../src/project/sigdb/reader'; import type { DecodedFunction } from '../../../src/project/sigdb/decode'; -import type { LibraryExports } from '../../../src/project/sigdb/schema'; +import { FnProp, type LibraryExports, type SigFunctionInfo } from '../../../src/project/sigdb/schema'; +import { RRange } from '../../../src/util/r-version'; +import { SigDbBuilder } from '../../../src/project/sigdb/build'; +import { sigTmpDir, writeAndOpen } from '../_helper/sigdb'; + +const fn = (name: string, opts: Partial = {}): SigFunctionInfo => ({ + name, props: FnProp.Exported, params: [], callees: [], line: 1, ...opts +}); /** a minimal in-memory signature source exposing a single, richly-decoded (and deprecated) function of `pkg` */ function sigDbWithDeprecatedFn(pkg: string, fnName: string): PackageSignatureSource { @@ -36,74 +44,227 @@ describe('flowR linter', withTreeSitter(parser => { /* Here, we expect no deprecated functions to be found, as neither `cat` nor `print` nor `<-` are listed as deprecated, we specifically clean the list of deprecated functions */ assertLinter('no function listed', parser, 'cat("hello")\nprint("hello")\nx <- 1\ncat(x)', 'deprecated-functions', [], - { totalCalls: 0, totalFunctionDefinitions: 0 }, - { fns: [] } + { hardcoded: 0, sigdb: 0 }, + { always: [] } ); /* Given that we declare `cat` as deprecated, we expect all uses to be marked! */ assertLinter('cat', parser, 'cat("hello")\nprint("hello")\nx <- 1\ncat(x)', 'deprecated-functions', [ - { certainty: LintingResultCertainty.Certain, function: 'cat', loc: [1, 1, 1, 12] }, - { certainty: LintingResultCertainty.Certain, function: 'cat', loc: [4, 1, 4, 6] }, + { certainty: LintingResultCertainty.Certain, function: 'base::cat', loc: [1, 1, 1, 12], type: 'deprecated-function' }, + { certainty: LintingResultCertainty.Certain, function: 'base::cat', loc: [4, 1, 4, 6], type: 'deprecated-function' }, ], - { totalCalls: 2, totalFunctionDefinitions: 2 }, - { fns: ['cat'] } + { hardcoded: 2, sigdb: 0 }, + { always: ['cat'] } ); /* Overwriting the `cat` function with a user defined implementation (even though it is useless), should cause the linter to not mark calls to the custom `cat` function as deprecated */ assertLinter('custom cat', parser, 'cat("hello")\nprint("hello")\ncat <- function(x) { }\nx <- 1\ncat(x)', 'deprecated-functions', [ - { certainty: LintingResultCertainty.Certain, function: 'cat', loc: [1, 1, 1, 12] } + { certainty: LintingResultCertainty.Certain, function: 'base::cat', loc: [1, 1, 1, 12], type: 'deprecated-function' } ], - { totalCalls: 1, totalFunctionDefinitions: 1 }, - { fns: ['cat'] } + { hardcoded: 1, sigdb: 0 }, + { always: ['cat'] } ); /* Using the default linter configuration, a function such as `all_equal` should be marked as deprecated */ assertLinter('with defaults', parser, 'all_equal(foo)', 'deprecated-functions', [ - { certainty: LintingResultCertainty.Certain, function: 'all_equal', loc: [1, 1, 1, 14] } + { certainty: LintingResultCertainty.Certain, function: 'all_equal', loc: [1, 1, 1, 14], type: 'deprecated-function' } ], - { totalCalls: 1, totalFunctionDefinitions: 1 } + { hardcoded: 1, sigdb: 0 } ); /* We should find deprecated functions even if they are nested in other function calls */ assertLinter('with defaults nested', parser, 'foo(all_equal(foo))', 'deprecated-functions', [ - { certainty: LintingResultCertainty.Certain, function: 'all_equal', loc: [1, 5, 1, 18] } + { certainty: LintingResultCertainty.Certain, function: 'all_equal', loc: [1, 5, 1, 18], type: 'deprecated-function' } ], - { totalCalls: 1, totalFunctionDefinitions: 1 } + { hardcoded: 1, sigdb: 0 } ); /* @ignore-in-wiki */ assertLinter('wiki example', parser, ` first <- data.frame(x = c(1, 2, 3), y = c(1, 2, 3)) second <- data.frame(x = c(1, 3, 2), y = c(1, 3, 2)) dplyr::all_equal(first, second)`, 'deprecated-functions', - [{ certainty: LintingResultCertainty.Certain, function: 'dplyr::all_equal', loc: [4, 1, 4, 31] }], - { totalCalls: 1, totalFunctionDefinitions: 1 }); + [{ certainty: LintingResultCertainty.Certain, function: 'dplyr::all_equal', loc: [4, 1, 4, 31], type: 'deprecated-function' }], + { hardcoded: 1, sigdb: 0 }); describe('a deprecated function resolved via a loaded package is still flagged', () => { // regression: the loaded-package export must still count as a built-in call target assertLinter('with a (controlled) package database', parser, 'library(dplyr)\nrecode(x)', 'deprecated-functions', - [{ certainty: LintingResultCertainty.Certain, function: 'recode', loc: [2, 1, 2, 9] }], - { totalCalls: 1, totalFunctionDefinitions: 1 }, - { fns: ['recode'], sigDb: controlledSigDb('dplyr', ['recode', 'filter']) } + [{ certainty: LintingResultCertainty.Certain, function: 'dplyr::recode', loc: [2, 1, 2, 9], type: 'deprecated-function' }], + { hardcoded: 1, sigdb: 0 }, + { always: ['recode'], sigDb: controlledSigDb('dplyr', ['recode', 'filter']) } ); assertLinter('without any package database', parser, 'library(dplyr)\nrecode(x)', 'deprecated-functions', - [{ certainty: LintingResultCertainty.Certain, function: 'recode', loc: [2, 1, 2, 9] }], - { totalCalls: 1, totalFunctionDefinitions: 1 }, - { fns: ['recode'], noSigDb: true } + [{ certainty: LintingResultCertainty.Certain, function: 'recode', loc: [2, 1, 2, 9], type: 'deprecated-function' }], + { hardcoded: 1, sigdb: 0 }, + { always: ['recode'], noSigDb: true } + ); + }); + + describe('only detect deprecated arg when value is set', () => { + assertLinter('deprecated arg but value not set', parser, 'testFn(badArg="hehe")', + 'deprecated-functions', + [], + { hardcoded: 0, sigdb: 0 }, + { always: [], conditionally: { 'testFn': { whenArgs: [{ argName: 'badArg', ifValue: 'not hehe', state: DeprecationState.Deprecated }] } } } + ); + + assertLinter('deprecated arg present', parser, 'testFn(badArg="not hehe")', + 'deprecated-functions', + [{ + type: 'deprecated-argument', + certainty: LintingResultCertainty.Certain, + arg: 'badArg', + replacedBy: undefined, + function: 'testFn', + state: DeprecationState.Deprecated, + sinceVersion: undefined, + loc: [1, 8, 1, 13] + }], + { hardcoded: 1, sigdb: 0 }, + { always: [], conditionally: { 'testFn': { whenArgs: [{ argName: 'badArg', ifValue: 'not hehe', state: DeprecationState.Deprecated }] } } } + ); + }); + + describe('only detect deprecated args when present', () => { + assertLinter('deprecated arg but not present', parser, 'testFn()', + 'deprecated-functions', + [], + { hardcoded: 0, sigdb: 0 }, + { always: [], conditionally: { 'testFn': { whenArgs: [{ argName: 'badArg', state: DeprecationState.Deprecated }] } } } + ); + + assertLinter('deprecated arg present', parser, 'testFn(badArg=5)', + 'deprecated-functions', + [{ + type: 'deprecated-argument', + certainty: LintingResultCertainty.Certain, + arg: 'badArg', + replacedBy: 'foo', + function: 'testFn', + state: DeprecationState.Deprecated, + sinceVersion: undefined, + loc: [1, 8, 1, 13] + }], + { hardcoded: 1, sigdb: 0 }, + { always: [], conditionally: { 'testFn': { whenArgs: [{ argName: 'badArg', state: DeprecationState.Deprecated, replacedBy: 'foo' }] } } } + ); + }); + + describe('only deprecate when version constraint is satisfied', async() => { + const b = new SigDbBuilder(); + b.addPackage('testPkg', { latest: '2.0.0', downloads: 5 }); + b.addVersion('testPkg', '2.0.0', { dependencies: [{ name: 'base', type: 1, constraint: '>= 1.0.0' }], cran: true, functions: [fn('testFn', { file: 'R/paste.R', line: 10, params: [ { name: 'badArg' } ] })] }); + b.addPackage('base', { latest: '4.5.3', core: true }); + const db = await writeAndOpen(sigTmpDir('dep-lint'), b.build({ date: '2026-05-23', generated: 0 })); + + assertLinter('(arg) unresolved version should make result uncertain', parser, 'library(testPkg)\ntestFn(badArg=5)', + 'deprecated-functions', + [{ + type: 'deprecated-argument', + certainty: LintingResultCertainty.Uncertain, + arg: 'badArg', + replacedBy: 'foo', + function: 'testFn', + state: DeprecationState.Deprecated, + sinceVersion: RRange.parse('>=1.0.0'), + loc: [2, 8, 2, 13] + }], + { hardcoded: 1, sigdb: 0 }, + { always: [], conditionally: { 'testFn': { package: 'testPkg', whenArgs: [{ argName: 'badArg', state: DeprecationState.Deprecated, replacedBy: 'foo', sinceVersion: RRange.parse('>=1.0.0') }] } } } + ); + + assertLinter('(arg) version resolved and constraint satisfied', parser, 'library(testPkg)\ntestFn(badArg=5)', + 'deprecated-functions', + [{ + type: 'deprecated-argument', + certainty: LintingResultCertainty.Certain, + arg: 'badArg', + replacedBy: 'foo', + function: 'testPkg::testFn', + state: DeprecationState.Deprecated, + sinceVersion: RRange.parse('>=1.0.0'), + loc: [2, 8, 2, 13] + }], + { hardcoded: 1, sigdb: 0 }, + { + always: [], + conditionally: { 'testFn': { package: 'testPkg', whenArgs: [{ argName: 'badArg', state: DeprecationState.Deprecated, replacedBy: 'foo', sinceVersion: RRange.parse('>=1.0.0') }] } }, + sigDb: db + } + ); + + assertLinter('(arg) version resolved and constraint not satisfied', parser, 'library(testPkg)\ntestFn(badArg=5)', + 'deprecated-functions', + [], + { hardcoded: 0, sigdb: 0 }, + { + always: [], + conditionally: { 'testFn': { package: 'testPkg', whenArgs: [{ argName: 'badArg', state: DeprecationState.Deprecated, replacedBy: 'foo', sinceVersion: RRange.parse('>=3.0.0') }] } }, + sigDb: db + } + ); + + + + assertLinter('(fn) unresolved version should make result uncertain', parser, 'library(testPkg)\ntestFn()', + 'deprecated-functions', + [{ + type: 'deprecated-function', + certainty: LintingResultCertainty.Uncertain, + function: 'testFn', + state: DeprecationState.Defunct, + sinceVersion: RRange.parse('>=1.0.0'), + replacedBy: undefined, + loc: [2, 1, 2, 8] + }], + { hardcoded: 1, sigdb: 0 }, + { always: [], conditionally: { 'testFn': { package: 'testPkg', sinceVersion: RRange.parse('>=1.0.0'), state: DeprecationState.Defunct } } } + ); + + assertLinter('(fn) version resolved and constraint satisfied', parser, 'library(testPkg)\ntestFn()', + 'deprecated-functions', + [{ + type: 'deprecated-function', + certainty: LintingResultCertainty.Certain, + function: 'testPkg::testFn', + state: DeprecationState.Defunct, + sinceVersion: RRange.parse('>=1.0.0'), + replacedBy: undefined, + loc: [2, 1, 2, 8] + }], + { hardcoded: 1, sigdb: 0 }, + { + always: [], + conditionally: { 'testFn': { package: 'testPkg', sinceVersion: RRange.parse('>=1.0.0'), state: DeprecationState.Defunct } }, + sigDb: db + } + ); + + + assertLinter('(fn) version resolved and constraint not satisfied', parser, 'library(testPkg)\ntestFn()', + 'deprecated-functions', + [], + { hardcoded: 0, sigdb: 0 }, + { + always: [], + conditionally: { 'testFn': { package: 'testPkg', sinceVersion: RRange.parse('>= 3.0.0'), state: DeprecationState.Defunct } }, + sigDb: db + } ); }); describe('a call the signature database marks deprecated is flagged even outside the hardcoded list', () => { assertLinter('sigdb-deprecated function not in fns', parser, 'library(dplyr)\nold_verb(x)', 'deprecated-functions', - [{ certainty: LintingResultCertainty.Certain, function: 'dplyr::old_verb', loc: [2, 1, 2, 11] }], - { totalCalls: 1, totalFunctionDefinitions: 1 }, + [{ type: 'deprecated-function', certainty: LintingResultCertainty.Certain, function: 'dplyr::old_verb', loc: [2, 1, 2, 11] }], + { hardcoded: 0, sigdb: 1 }, { fns: [], sigDb: sigDbWithDeprecatedFn('dplyr', 'old_verb') } ); assertLinter('not flagged without a package database', parser, 'library(dplyr)\nold_verb(x)', 'deprecated-functions', [], - { totalCalls: 0, totalFunctionDefinitions: 0 }, + { hardcoded: 0, sigdb: 0 }, { fns: [], noSigDb: true } ); });