From c0979743d0a93b39fe99e452149b240330b78ab8 Mon Sep 17 00:00:00 2001 From: gigalasr Date: Thu, 9 Jul 2026 15:17:46 +0200 Subject: [PATCH 1/7] feat(lint): add structure for deprectaed args - #1870 --- src/linter/rules/deprecated-functions.ts | 57 ++++++++++++++++++++++-- src/linter/rules/network-functions.ts | 8 ++-- 2 files changed, 56 insertions(+), 9 deletions(-) diff --git a/src/linter/rules/deprecated-functions.ts b/src/linter/rules/deprecated-functions.ts index 6ff68da6c2b..f2559477fe7 100644 --- a/src/linter/rules/deprecated-functions.ts +++ b/src/linter/rules/deprecated-functions.ts @@ -2,6 +2,48 @@ import { LintingRuleCertainty, type LintingRule } from '../linter-format'; import { LintingRuleTag } from '../linter-tags'; import { type FunctionsMetadata, type FunctionsResult, type FunctionsToDetectConfig, functionFinderUtil } from './function-finder-util'; +/** + * Information about an argument of a function that should be flagged as deprected if it is called with this argument + */ +export interface DeprecatedArgumentInformation { + argIdx?: number, + argName?: string + ifValue?: RegExp | string + replacedBy?: string + version?: string + state: DeprecationState +} + +export const enum DeprecationState { + /** + * Still works, but marked for removal + */ + Deprecated, + /** + * No longer works, and marked for removal + */ + Defunct, + /** + * Replaced by another function but kept for compatibility + */ + Superseeded +} + +export interface DeprecatedFunctionsConfig extends FunctionsToDetectConfig { + /** + * Functions to mark as deprecated + */ + fns: Fns + /** + * Additionally, constraint the detection of an fn in {@link DeprecatedFunctionsConfig.fns} if it is called with an Argument + * defined by {@link DeprecatedFunctionsConfig.whenArgs} + */ + whenArgs: Partial> +} +// Little helper for constraining whenArgs to fns +const defineConfig = (config: DeprecatedFunctionsConfig) => config; + + export const DEPRECATED_FUNCTIONS = { createSearch: (config) => functionFinderUtil.createSearch(config.fns), processSearchResult: functionFinderUtil.processSearchResult, @@ -12,8 +54,15 @@ export const DEPRECATED_FUNCTIONS = { // ensures all deprecated functions found are actually deprecated through its limited config, but doesn't find all deprecated functions since the config is pre-crawled 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'] - } + defaultConfig: defineConfig({ + fns: ['geom_violin', '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'], + whenArgs: { + 'geom_violin': [{ + argName: 'draw_quantiles', + state: DeprecationState.Deprecated, + replacedBy: 'quantile.linetype' + }] + } + }) } -} as const satisfies LintingRule; +} as const satisfies LintingRule; diff --git a/src/linter/rules/network-functions.ts b/src/linter/rules/network-functions.ts index 9431b3d0aeb..e34cc5972af 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 { FunctionsToDetectConfig, 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'; @@ -9,9 +9,7 @@ import { Ternary } from '../../util/logic'; import { SourceFunctions } from '../../queries/catalog/dependencies-query/function-info/source-functions'; import { WriteFunctions } from '../../queries/catalog/dependencies-query/function-info/write-functions'; -export interface NetworkFunctionsConfig extends MergeableRecord { - /** The list of function names that should be marked in the given context if their arguments match. */ - fns: readonly string[] +export interface NetworkFunctionsConfig extends FunctionsToDetectConfig { /** only trigger if the function's read argument is linked to a value that matches this pattern */ onlyTriggerWithArgument?: RegExp | string } From 9116b6ef2b03d11e85ea8982fb2fdba851dcb93a Mon Sep 17 00:00:00 2001 From: gigalasr Date: Thu, 9 Jul 2026 17:39:53 +0200 Subject: [PATCH 2/7] feat(lint): only detect when deprectaed arg is present - #1870 --- src/linter/rules/deprecated-functions.ts | 77 +++++++++++++++++-- src/linter/rules/naming-convention.ts | 2 +- .../linter/lint-deprecated-functions.test.ts | 21 +++++ 3 files changed, 94 insertions(+), 6 deletions(-) diff --git a/src/linter/rules/deprecated-functions.ts b/src/linter/rules/deprecated-functions.ts index f2559477fe7..8c11aea248f 100644 --- a/src/linter/rules/deprecated-functions.ts +++ b/src/linter/rules/deprecated-functions.ts @@ -1,4 +1,10 @@ -import { LintingRuleCertainty, type LintingRule } from '../linter-format'; +import type { BrandedIdentifier } from '../../dataflow/environments/identifier'; +import { FunctionArgument } from '../../dataflow/graph/graph'; +import { isFunctionCallVertex } from '../../dataflow/graph/vertex'; +import { Enrichment, enrichmentContent } from '../../search/search-executor/search-enrichers'; +import { isNotUndefined } from '../../util/assert'; +import { SourceLocation } from '../../util/range'; +import { LintingResultCertainty, LintingRuleCertainty, type LintingRule } from '../linter-format'; import { LintingRuleTag } from '../linter-tags'; import { type FunctionsMetadata, type FunctionsResult, type FunctionsToDetectConfig, functionFinderUtil } from './function-finder-util'; @@ -43,12 +49,73 @@ export interface DeprecatedFunctionsConfig(config: DeprecatedFunctionsConfig) => config; - export const DEPRECATED_FUNCTIONS = { createSearch: (config) => functionFinderUtil.createSearch(config.fns), - processSearchResult: functionFinderUtil.processSearchResult, - prettyPrint: functionFinderUtil.prettyPrint('deprecated'), - info: { + processSearchResult: async(elements, config, data) => { + const dataflow = (await data.dataflow()).graph; + const metadata: FunctionsMetadata = { + totalCalls: 0, + totalFunctionDefinitions: 0 + }; + + const results = elements.getElements().flatMap(e => { + metadata.totalCalls++; + return enrichmentContent(e, Enrichment.CallTargets).targets.map(target => { + metadata.totalFunctionDefinitions++; + return { + node: e.node, + loc: SourceLocation.fromNode(e.node), + target: target as BrandedIdentifier, + }; + }); + }).filter(e => isNotUndefined(e.loc)); + + // Filter out functions that are only deprecated when a certain Argument is present + const resultsWhenArg = results.flatMap(r => { + // If not provided in whenArgs, it is always deprecated + const whenArgs = config.whenArgs[r.target]; + if(whenArgs === undefined) { + return r; + } + + // Check if function has deprecated args + const deprecatedArgs = whenArgs.map(depricationInfo => { + const vertex = dataflow.getVertex(r.node.info.id); + if(vertex === undefined || !isFunctionCallVertex(vertex)) { + return undefined; + } + + if(vertex.args.some((arg, idx) => + FunctionArgument.isNamed(arg) && arg.name === depricationInfo.argName || + FunctionArgument.isPositional(arg) && idx === depricationInfo.argIdx + )) { + return { + arg: depricationInfo.argName ?? depricationInfo.argIdx, + state: depricationInfo.state, + replacedBy: depricationInfo.replacedBy + }; + } + }).filter(p => p !== undefined); + + // Don't mark function as deprecated, if we didn't find any deprecated args + return deprecatedArgs.length === 0 ? undefined : { + ...r, + deprecatedArgs: deprecatedArgs + }; + }).filter(p => p !== undefined); + + return { + results: resultsWhenArg.map(e => ({ + certainty: LintingResultCertainty.Certain, + involvedId: e.node.info.id, + function: e.target, + loc: e.loc + })) as FunctionsResult[], + '.meta': metadata + }; + }, + prettyPrint: functionFinderUtil.prettyPrint('deprecated'), + info: { name: 'Deprecated Functions', tags: [LintingRuleTag.Deprecated, LintingRuleTag.Smell, LintingRuleTag.Usability, LintingRuleTag.Reproducibility], // ensures all deprecated functions found are actually deprecated through its limited config, but doesn't find all deprecated functions since the config is pre-crawled diff --git a/src/linter/rules/naming-convention.ts b/src/linter/rules/naming-convention.ts index 30a6a3b5d5b..a8a4e4b484f 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/test/functionality/linter/lint-deprecated-functions.test.ts b/test/functionality/linter/lint-deprecated-functions.test.ts index f8e8550c449..6fb0849a2b9 100644 --- a/test/functionality/linter/lint-deprecated-functions.test.ts +++ b/test/functionality/linter/lint-deprecated-functions.test.ts @@ -2,6 +2,7 @@ import { describe } from 'vitest'; import { withTreeSitter } from '../_helper/shell'; import { assertLinter, controlledPkgDb } from '../_helper/linter'; import { LintingResultCertainty } from '../../../src/linter/linter-format'; +import { DeprecationState } from '../../../src/linter/rules/deprecated-functions'; describe('flowR linter', withTreeSitter(parser => { describe('deprecated functions', () => { @@ -65,5 +66,25 @@ dplyr::all_equal(first, second)`, 'deprecated-functions', { fns: ['recode'], noPkgDb: true } ); }); + + describe('only detect deprecated args when present', () => { + assertLinter('deprecated arg but not present', parser, 'testFn()', + 'deprecated-functions', + [], + { totalCalls: 1, totalFunctionDefinitions: 1 }, + { fns: ['testFn'], whenArgs: { 'testFn': [{ argName: 'badArg', state: DeprecationState.Deprecated }] } } + ); + + assertLinter('deprecated arg present', parser, 'testFn(badArg=5)', + 'deprecated-functions', + [{ + certainty: LintingResultCertainty.Certain, + function: 'testFn', + loc: [1, 1, 1, 16] + }], + { totalCalls: 1, totalFunctionDefinitions: 1 }, + { fns: ['testFn'], whenArgs: { 'testFn': [{ argName: 'badArg', state: DeprecationState.Deprecated }] } } + ); + }); }); })); From 05e92b888d2b834d42f4e204df602a762ddb3ba7 Mon Sep 17 00:00:00 2001 From: gigalasr Date: Fri, 10 Jul 2026 12:06:17 +0200 Subject: [PATCH 3/7] feat(lint): allow new metadata in functions as well - #1870 --- src/linter/rules/deprecated-functions.ts | 193 +++++++++++------- .../linter/lint-deprecated-functions.test.ts | 30 +-- 2 files changed, 136 insertions(+), 87 deletions(-) diff --git a/src/linter/rules/deprecated-functions.ts b/src/linter/rules/deprecated-functions.ts index 8c11aea248f..88a9136d5e3 100644 --- a/src/linter/rules/deprecated-functions.ts +++ b/src/linter/rules/deprecated-functions.ts @@ -1,25 +1,59 @@ import type { BrandedIdentifier } from '../../dataflow/environments/identifier'; import { FunctionArgument } from '../../dataflow/graph/graph'; import { isFunctionCallVertex } 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 { LintingResultCertainty, LintingRuleCertainty, type LintingRule } from '../linter-format'; +import type { LintingResult, LintingRule } from '../linter-format'; +import { LintingResultCertainty, LintingRuleCertainty } from '../linter-format'; import { LintingRuleTag } from '../linter-tags'; -import { type FunctionsMetadata, type FunctionsResult, type FunctionsToDetectConfig, functionFinderUtil } from './function-finder-util'; +import { type FunctionsMetadata, functionFinderUtil } from './function-finder-util'; /** - * Information about an argument of a function that should be flagged as deprected if it is called with this argument + * Information about an argument of a function that should be flagged as deprecated if it is called with this argument */ export interface DeprecatedArgumentInformation { - argIdx?: number, - argName?: string - ifValue?: RegExp | string + argIdx?: number, + argName?: string + ifValue?: RegExp | string + replacedBy?: string + sinceVersion?: string + state?: DeprecationState +} + +/** + * Information about a deprecated function + */ +export interface DeprecatedFunctionInformation { + name: string + /** Used to mark specific arguments as deprecated */ + whenArgs?: DeprecatedArgumentInformation[] + replacedBy?: string + sinceVersion?: string + state?: DeprecationState +} + +export interface DeprecatedFunctionResultBase extends LintingResult { + type: 'deprecated-function' | 'deprecated-arg' + function: BrandedIdentifier replacedBy?: string - version?: string - state: DeprecationState + state?: DeprecationState +} + +export interface DeprecatedFunctionResult extends DeprecatedFunctionResultBase { + type: 'deprecated-function' } +export interface DeprecatedArgumentResult extends DeprecatedFunctionResultBase { + type: 'deprecated-arg' + arg: string | number +} + +export type DeprecatedFunctionLintingResult = DeprecatedFunctionResult | DeprecatedArgumentResult; + + export const enum DeprecationState { /** * Still works, but marked for removal @@ -35,83 +69,103 @@ export const enum DeprecationState { Superseeded } -export interface DeprecatedFunctionsConfig extends FunctionsToDetectConfig { +export interface DeprecatedFunctionsConfig extends MergeableRecord{ /** * Functions to mark as deprecated */ - fns: Fns - /** - * Additionally, constraint the detection of an fn in {@link DeprecatedFunctionsConfig.fns} if it is called with an Argument - * defined by {@link DeprecatedFunctionsConfig.whenArgs} - */ - whenArgs: Partial> + fns: (string | DeprecatedFunctionInformation)[] } -// Little helper for constraining whenArgs to fns -const defineConfig = (config: DeprecatedFunctionsConfig) => config; + + +const DeprecatedFunctions = [ + { name: 'geom_violin', whenArgs: [{ argName: 'draw_quantiles', state: DeprecationState.Deprecated, replacedBy: 'quantile.linetype' }] }, + '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' +] satisfies (string | DeprecatedFunctionInformation)[]; export const DEPRECATED_FUNCTIONS = { - createSearch: (config) => functionFinderUtil.createSearch(config.fns), + createSearch: (config) => functionFinderUtil.createSearch(config.fns.map(fn => typeof fn === 'string' ? fn : fn.name)), processSearchResult: async(elements, config, data) => { + const fnInfosEx = Object.fromEntries( + config.fns + .filter(f => typeof f === 'object') + .map(f => [f.name, f]) + ); + const dataflow = (await data.dataflow()).graph; + const metadata: FunctionsMetadata = { totalCalls: 0, totalFunctionDefinitions: 0 }; - const results = elements.getElements().flatMap(e => { + const detectedFunctions = elements.getElements().flatMap(e => { metadata.totalCalls++; return enrichmentContent(e, Enrichment.CallTargets).targets.map(target => { metadata.totalFunctionDefinitions++; - return { - node: e.node, - loc: SourceLocation.fromNode(e.node), - target: target as BrandedIdentifier, - }; - }); - }).filter(e => isNotUndefined(e.loc)); - - // Filter out functions that are only deprecated when a certain Argument is present - const resultsWhenArg = results.flatMap(r => { - // If not provided in whenArgs, it is always deprecated - const whenArgs = config.whenArgs[r.target]; - if(whenArgs === undefined) { - return r; - } - - // Check if function has deprecated args - const deprecatedArgs = whenArgs.map(depricationInfo => { - const vertex = dataflow.getVertex(r.node.info.id); - if(vertex === undefined || !isFunctionCallVertex(vertex)) { - return undefined; - } - - if(vertex.args.some((arg, idx) => - FunctionArgument.isNamed(arg) && arg.name === depricationInfo.argName || - FunctionArgument.isPositional(arg) && idx === depricationInfo.argIdx - )) { + const sourceLocation = SourceLocation.fromNode(e.node); + if(sourceLocation !== undefined) { return { - arg: depricationInfo.argName ?? depricationInfo.argIdx, - state: depricationInfo.state, - replacedBy: depricationInfo.replacedBy + node: e.node, target: target as BrandedIdentifier, sourceLocation }; } - }).filter(p => p !== undefined); + }); + }).filter(p => isNotUndefined(p)); + + const results = detectedFunctions.map(e => { + const info = fnInfosEx[e.target]; + if(info === undefined) { + // No extra info => function is always marked as deprecated + return [{ + type: 'deprecated-function', + certainty: LintingResultCertainty.Certain, + involvedId: e.node.info.id, + loc: e.sourceLocation, + function: e.target, + } satisfies DeprecatedFunctionResult]; + } else if(info.whenArgs !== undefined) { + // If whenArgs is set, the function should only be deprecated if it has one of the provided args + return info.whenArgs.map(deprecatedArgInfo => { + const vertex = dataflow.getVertex(e.node.info.id); + if(vertex === undefined || !isFunctionCallVertex(vertex)) { + return undefined; + } + + const arg = vertex.args.find((arg, idx) => + FunctionArgument.isNamed(arg) && arg.name === deprecatedArgInfo.argName || + FunctionArgument.isPositional(arg) && idx === deprecatedArgInfo.argIdx + ); - // Don't mark function as deprecated, if we didn't find any deprecated args - return deprecatedArgs.length === 0 ? undefined : { - ...r, - deprecatedArgs: deprecatedArgs - }; - }).filter(p => p !== undefined); + if(arg !== undefined) { + return { + type: 'deprecated-arg', + certainty: LintingResultCertainty.Certain, + involvedId: arg === EmptyArgument ? e.node.info.id : arg.nodeId, + function: e.target, + arg: (deprecatedArgInfo.argName ?? deprecatedArgInfo.argIdx) as string | number, + state: deprecatedArgInfo.state, + replacedBy: deprecatedArgInfo.replacedBy, + loc: e.sourceLocation + } satisfies DeprecatedArgumentResult; + } + + // Don't mark function as deprecated, if we didn't find any deprecated args + }).filter(p => p !== undefined); + } else { + // Extra Info but no whenArgs => always marked as deprecated and extra info is provided + return [{ + type: 'deprecated-function', + certainty: LintingResultCertainty.Certain, + involvedId: e.node.info.id, + loc: e.sourceLocation, + function: e.target, + state: info?.state, + replacedBy: info?.replacedBy + } satisfies DeprecatedFunctionResult]; + } + }).flat(); return { - results: resultsWhenArg.map(e => ({ - certainty: LintingResultCertainty.Certain, - involvedId: e.node.info.id, - function: e.target, - loc: e.loc - })) as FunctionsResult[], - '.meta': metadata + results, '.meta': metadata }; }, prettyPrint: functionFinderUtil.prettyPrint('deprecated'), @@ -121,15 +175,6 @@ export const DEPRECATED_FUNCTIONS = { // ensures all deprecated functions found are actually deprecated through its limited config, but doesn't find all deprecated functions since the config is pre-crawled certainty: LintingRuleCertainty.BestEffort, description: 'Marks deprecated functions that should not be used anymore.', - defaultConfig: defineConfig({ - fns: ['geom_violin', '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'], - whenArgs: { - 'geom_violin': [{ - argName: 'draw_quantiles', - state: DeprecationState.Deprecated, - replacedBy: 'quantile.linetype' - }] - } - }) + defaultConfig: { fns: DeprecatedFunctions } } -} as const satisfies LintingRule; +} as const satisfies LintingRule; diff --git a/test/functionality/linter/lint-deprecated-functions.test.ts b/test/functionality/linter/lint-deprecated-functions.test.ts index 6fb0849a2b9..d8c6515393b 100644 --- a/test/functionality/linter/lint-deprecated-functions.test.ts +++ b/test/functionality/linter/lint-deprecated-functions.test.ts @@ -15,8 +15,8 @@ describe('flowR linter', withTreeSitter(parser => { /* 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: 'cat', loc: [1, 1, 1, 12], type: 'deprecated-function' }, + { certainty: LintingResultCertainty.Certain, function: 'cat', loc: [4, 1, 4, 6], type: 'deprecated-function' }, ], { totalCalls: 2, totalFunctionDefinitions: 2 }, { fns: ['cat'] } @@ -24,7 +24,7 @@ describe('flowR linter', withTreeSitter(parser => { /* 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: 'cat', loc: [1, 1, 1, 12], type: 'deprecated-function' } ], { totalCalls: 1, totalFunctionDefinitions: 1 }, { fns: ['cat'] } @@ -32,14 +32,14 @@ describe('flowR linter', withTreeSitter(parser => { /* 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 } ); /* 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 } ); @@ -48,20 +48,20 @@ describe('flowR linter', withTreeSitter(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] }], + [{ certainty: LintingResultCertainty.Certain, function: 'dplyr::all_equal', loc: [4, 1, 4, 31], type: 'deprecated-function' }], { totalCalls: 1, totalFunctionDefinitions: 1 }); 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] }], + [{ certainty: LintingResultCertainty.Certain, function: 'recode', loc: [2, 1, 2, 9], type: 'deprecated-function' }], { totalCalls: 1, totalFunctionDefinitions: 1 }, { fns: ['recode'], pkgDb: controlledPkgDb('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] }], + [{ certainty: LintingResultCertainty.Certain, function: 'recode', loc: [2, 1, 2, 9], type: 'deprecated-function' }], { totalCalls: 1, totalFunctionDefinitions: 1 }, { fns: ['recode'], noPkgDb: true } ); @@ -72,18 +72,22 @@ dplyr::all_equal(first, second)`, 'deprecated-functions', 'deprecated-functions', [], { totalCalls: 1, totalFunctionDefinitions: 1 }, - { fns: ['testFn'], whenArgs: { 'testFn': [{ argName: 'badArg', state: DeprecationState.Deprecated }] } } + { fns: [{ name: 'testFn', whenArgs: [{ argName: 'badArg', state: DeprecationState.Deprecated }] } ] } ); assertLinter('deprecated arg present', parser, 'testFn(badArg=5)', 'deprecated-functions', [{ - certainty: LintingResultCertainty.Certain, - function: 'testFn', - loc: [1, 1, 1, 16] + type: 'deprecated-arg', + certainty: LintingResultCertainty.Certain, + arg: 'badArg', + replacedBy: 'foo', + state: DeprecationState.Deprecated, + function: 'testFn', + loc: [1, 1, 1, 16] }], { totalCalls: 1, totalFunctionDefinitions: 1 }, - { fns: ['testFn'], whenArgs: { 'testFn': [{ argName: 'badArg', state: DeprecationState.Deprecated }] } } + { fns: [{ name: 'testFn', whenArgs: [{ argName: 'badArg', state: DeprecationState.Deprecated, replacedBy: 'foo' }] } ] } ); }); }); From a43d0bc0e67495e3a34d0e4447729cad302b99fa Mon Sep 17 00:00:00 2001 From: gigalasr Date: Fri, 10 Jul 2026 14:14:44 +0200 Subject: [PATCH 4/7] feat(lint): pretty print new deprecation info - #1870 --- src/linter/rules/deprecated-functions.ts | 103 ++++++++++-------- test/functionality/_helper/linter.ts | 4 +- .../linter/lint-deprecated-functions.test.ts | 18 +-- 3 files changed, 70 insertions(+), 55 deletions(-) diff --git a/src/linter/rules/deprecated-functions.ts b/src/linter/rules/deprecated-functions.ts index 88a9136d5e3..dfab1d1f5a0 100644 --- a/src/linter/rules/deprecated-functions.ts +++ b/src/linter/rules/deprecated-functions.ts @@ -1,3 +1,4 @@ +import type { Range } from 'semver'; import type { BrandedIdentifier } from '../../dataflow/environments/identifier'; import { FunctionArgument } from '../../dataflow/graph/graph'; import { isFunctionCallVertex } from '../../dataflow/graph/vertex'; @@ -7,9 +8,10 @@ import { isNotUndefined } from '../../util/assert'; import type { MergeableRecord } from '../../util/objects'; import { SourceLocation } from '../../util/range'; import type { LintingResult, LintingRule } from '../linter-format'; -import { LintingResultCertainty, LintingRuleCertainty } from '../linter-format'; +import { LintingPrettyPrintContext, LintingResultCertainty, LintingRuleCertainty } from '../linter-format'; import { LintingRuleTag } from '../linter-tags'; import { type FunctionsMetadata, functionFinderUtil } from './function-finder-util'; +import { parseRRange } from '../../util/r-version'; /** * Information about an argument of a function that should be flagged as deprecated if it is called with this argument @@ -19,7 +21,7 @@ export interface DeprecatedArgumentInformation { argName?: string ifValue?: RegExp | string replacedBy?: string - sinceVersion?: string + sinceVersion?: Range state?: DeprecationState } @@ -31,15 +33,15 @@ export interface DeprecatedFunctionInformation { /** Used to mark specific arguments as deprecated */ whenArgs?: DeprecatedArgumentInformation[] replacedBy?: string - sinceVersion?: string + sinceVersion?: Range state?: DeprecationState } export interface DeprecatedFunctionResultBase extends LintingResult { - type: 'deprecated-function' | 'deprecated-arg' - function: BrandedIdentifier - replacedBy?: string - state?: DeprecationState + function: BrandedIdentifier + replacedBy?: string + sinceVersion?: Range + state?: DeprecationState } export interface DeprecatedFunctionResult extends DeprecatedFunctionResultBase { @@ -47,38 +49,29 @@ export interface DeprecatedFunctionResult extends DeprecatedFunctionResultBase { } export interface DeprecatedArgumentResult extends DeprecatedFunctionResultBase { - type: 'deprecated-arg' + type: 'deprecated-argument' arg: string | number } -export type DeprecatedFunctionLintingResult = DeprecatedFunctionResult | DeprecatedArgumentResult; - - -export const enum DeprecationState { - /** - * Still works, but marked for removal - */ - Deprecated, - /** - * No longer works, and marked for removal - */ - Defunct, - /** - * Replaced by another function but kept for compatibility - */ - Superseeded +export type DeprecatedFunctionRuleResult = DeprecatedFunctionResult | DeprecatedArgumentResult; + +export enum DeprecationState { + /** Still works, but marked for removal */ + Deprecated = 'deprecated', + /** No longer works, and marked for removal */ + Defunct = 'defunct', + /** Replaced by another function but kept for compatibility */ + Superseeded = 'superseeded' } export interface DeprecatedFunctionsConfig extends MergeableRecord{ - /** - * Functions to mark as deprecated - */ + /** Functions to mark as deprecated */ fns: (string | DeprecatedFunctionInformation)[] } const DeprecatedFunctions = [ - { name: 'geom_violin', whenArgs: [{ argName: 'draw_quantiles', state: DeprecationState.Deprecated, replacedBy: 'quantile.linetype' }] }, + { name: 'geom_violin', whenArgs: [{ argName: 'draw_quantiles', state: DeprecationState.Deprecated, replacedBy: 'quantile.linetype', sinceVersion: parseRRange('>= 4.0.0') }] }, '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' ] satisfies (string | DeprecatedFunctionInformation)[]; @@ -137,14 +130,15 @@ export const DEPRECATED_FUNCTIONS = { if(arg !== undefined) { return { - type: 'deprecated-arg', - certainty: LintingResultCertainty.Certain, - involvedId: arg === EmptyArgument ? e.node.info.id : arg.nodeId, - function: e.target, - arg: (deprecatedArgInfo.argName ?? deprecatedArgInfo.argIdx) as string | number, - state: deprecatedArgInfo.state, - replacedBy: deprecatedArgInfo.replacedBy, - loc: e.sourceLocation + type: 'deprecated-argument', + certainty: LintingResultCertainty.Certain, + involvedId: arg === EmptyArgument ? e.node.info.id : arg.nodeId, + function: e.target, + arg: (deprecatedArgInfo.argName ?? deprecatedArgInfo.argIdx) as string | number, + state: deprecatedArgInfo.state, + replacedBy: deprecatedArgInfo.replacedBy, + sinceVersion: deprecatedArgInfo.sinceVersion, + loc: e.sourceLocation } satisfies DeprecatedArgumentResult; } @@ -153,13 +147,14 @@ export const DEPRECATED_FUNCTIONS = { } else { // Extra Info but no whenArgs => always marked as deprecated and extra info is provided return [{ - type: 'deprecated-function', - certainty: LintingResultCertainty.Certain, - involvedId: e.node.info.id, - loc: e.sourceLocation, - function: e.target, - state: info?.state, - replacedBy: info?.replacedBy + type: 'deprecated-function', + certainty: LintingResultCertainty.Certain, + involvedId: e.node.info.id, + loc: e.sourceLocation, + function: e.target, + state: info.state, + replacedBy: info.replacedBy, + sinceVersion: info.sinceVersion } satisfies DeprecatedFunctionResult]; } }).flat(); @@ -168,8 +163,24 @@ export const DEPRECATED_FUNCTIONS = { results, '.meta': metadata }; }, - prettyPrint: functionFinderUtil.prettyPrint('deprecated'), - info: { + prettyPrint: { + [LintingPrettyPrintContext.Query]: (result: DeprecatedFunctionRuleResult) => `${result.type === 'deprecated-argument' ? `Argument \`${result.arg}\` of ` : ''}Function \`${result.function}\` at ${SourceLocation.format(result.loc)}`, + [LintingPrettyPrintContext.Full]: (result: DeprecatedFunctionRuleResult) => { + const str: string[] = []; + if(result.type === 'deprecated-argument') { + str.push(`Argument \`${result.arg}\` of`); + } + str.push(`Function \`${result.function}\` is ${result.state ?? 'deprecated'}`); + if(result.sinceVersion) { + str.push(`since version ${result.sinceVersion.format()}`); + } + if(result.replacedBy) { + str.push(`and replaced by \`${result.replacedBy}\``); + } + return str.join(' '); + } + }, + info: { name: 'Deprecated Functions', tags: [LintingRuleTag.Deprecated, LintingRuleTag.Smell, LintingRuleTag.Usability, LintingRuleTag.Reproducibility], // ensures all deprecated functions found are actually deprecated through its limited config, but doesn't find all deprecated functions since the config is pre-crawled @@ -177,4 +188,4 @@ export const DEPRECATED_FUNCTIONS = { description: 'Marks deprecated functions that should not be used anymore.', defaultConfig: { fns: DeprecatedFunctions } } -} as const satisfies LintingRule; +} as const satisfies LintingRule; diff --git a/test/functionality/_helper/linter.ts b/test/functionality/_helper/linter.ts index d249f8409e8..f67eaabd4e8 100644 --- a/test/functionality/_helper/linter.ts +++ b/test/functionality/_helper/linter.ts @@ -44,6 +44,8 @@ export function controlledPkgDb(pkg: string, exports: readonly string[]): PkgDb } +type DistributiveOmit = T extends unknown ? Omit : never; + /** * Asserts correct linting results while ignoring each linting result's {@link LintingRuleResult.involvedId}. */ @@ -52,7 +54,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 d8c6515393b..41a65f1e1c2 100644 --- a/test/functionality/linter/lint-deprecated-functions.test.ts +++ b/test/functionality/linter/lint-deprecated-functions.test.ts @@ -3,6 +3,7 @@ import { withTreeSitter } from '../_helper/shell'; import { assertLinter, controlledPkgDb } from '../_helper/linter'; import { LintingResultCertainty } from '../../../src/linter/linter-format'; import { DeprecationState } from '../../../src/linter/rules/deprecated-functions'; +import { parseRRange } from '../../../src/util/r-version'; describe('flowR linter', withTreeSitter(parser => { describe('deprecated functions', () => { @@ -78,16 +79,17 @@ dplyr::all_equal(first, second)`, 'deprecated-functions', assertLinter('deprecated arg present', parser, 'testFn(badArg=5)', 'deprecated-functions', [{ - type: 'deprecated-arg', - certainty: LintingResultCertainty.Certain, - arg: 'badArg', - replacedBy: 'foo', - state: DeprecationState.Deprecated, - function: 'testFn', - loc: [1, 1, 1, 16] + type: 'deprecated-argument', + certainty: LintingResultCertainty.Certain, + arg: 'badArg', + replacedBy: 'foo', + function: 'testFn', + state: DeprecationState.Deprecated, + sinceVersion: parseRRange('>= 4.0.0'), + loc: [1, 1, 1, 16] }], { totalCalls: 1, totalFunctionDefinitions: 1 }, - { fns: [{ name: 'testFn', whenArgs: [{ argName: 'badArg', state: DeprecationState.Deprecated, replacedBy: 'foo' }] } ] } + { fns: [{ name: 'testFn', whenArgs: [{ argName: 'badArg', state: DeprecationState.Deprecated, replacedBy: 'foo', sinceVersion: parseRRange('>= 4.0.0') }] } ] } ); }); }); From 493606b3aaa39f2738a741c33a3e86b677597c25 Mon Sep 17 00:00:00 2001 From: gigalasr Date: Fri, 10 Jul 2026 14:26:13 +0200 Subject: [PATCH 5/7] feat(lint): use source location of arg - #1870 --- src/linter/rules/deprecated-functions.ts | 13 ++++++++----- .../linter/lint-deprecated-functions.test.ts | 2 +- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/linter/rules/deprecated-functions.ts b/src/linter/rules/deprecated-functions.ts index dfab1d1f5a0..fade1db13cd 100644 --- a/src/linter/rules/deprecated-functions.ts +++ b/src/linter/rules/deprecated-functions.ts @@ -85,6 +85,7 @@ export const DEPRECATED_FUNCTIONS = { ); const dataflow = (await data.dataflow()).graph; + const idMap = (await data.normalize()).idMap; const metadata: FunctionsMetadata = { totalCalls: 0, @@ -127,18 +128,19 @@ export const DEPRECATED_FUNCTIONS = { 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(arg !== undefined) { + if(argNode !== undefined) { return { type: 'deprecated-argument', certainty: LintingResultCertainty.Certain, - involvedId: arg === EmptyArgument ? e.node.info.id : arg.nodeId, + involvedId: argNode.info.id, function: e.target, arg: (deprecatedArgInfo.argName ?? deprecatedArgInfo.argIdx) as string | number, state: deprecatedArgInfo.state, replacedBy: deprecatedArgInfo.replacedBy, sinceVersion: deprecatedArgInfo.sinceVersion, - loc: e.sourceLocation + loc: SourceLocation.fromNode(argNode) ?? e.sourceLocation } satisfies DeprecatedArgumentResult; } @@ -168,14 +170,15 @@ export const DEPRECATED_FUNCTIONS = { [LintingPrettyPrintContext.Full]: (result: DeprecatedFunctionRuleResult) => { const str: string[] = []; if(result.type === 'deprecated-argument') { - str.push(`Argument \`${result.arg}\` of`); + const argStr = typeof result.arg === 'number' ? `at position \`${result.arg}\`` : result.arg; + str.push(`Argument \`${argStr}\` of`); } str.push(`Function \`${result.function}\` is ${result.state ?? 'deprecated'}`); if(result.sinceVersion) { str.push(`since version ${result.sinceVersion.format()}`); } if(result.replacedBy) { - str.push(`and replaced by \`${result.replacedBy}\``); + str.push(`and is replaced by \`${result.replacedBy}\``); } return str.join(' '); } diff --git a/test/functionality/linter/lint-deprecated-functions.test.ts b/test/functionality/linter/lint-deprecated-functions.test.ts index 41a65f1e1c2..31d67fe3e13 100644 --- a/test/functionality/linter/lint-deprecated-functions.test.ts +++ b/test/functionality/linter/lint-deprecated-functions.test.ts @@ -86,7 +86,7 @@ dplyr::all_equal(first, second)`, 'deprecated-functions', function: 'testFn', state: DeprecationState.Deprecated, sinceVersion: parseRRange('>= 4.0.0'), - loc: [1, 1, 1, 16] + loc: [1, 8, 1, 13] }], { totalCalls: 1, totalFunctionDefinitions: 1 }, { fns: [{ name: 'testFn', whenArgs: [{ argName: 'badArg', state: DeprecationState.Deprecated, replacedBy: 'foo', sinceVersion: parseRRange('>= 4.0.0') }] } ] } From b4a79ba033637afa3c30bde7e3a541fab183348b Mon Sep 17 00:00:00 2001 From: gigalasr Date: Thu, 16 Jul 2026 11:21:21 +0200 Subject: [PATCH 6/7] feat-fix(lint): use new RRange util - #1870 --- src/linter/rules/deprecated-functions.ts | 4 ++-- test/functionality/linter/lint-deprecated-functions.test.ts | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/linter/rules/deprecated-functions.ts b/src/linter/rules/deprecated-functions.ts index fade1db13cd..1c6cbb861b3 100644 --- a/src/linter/rules/deprecated-functions.ts +++ b/src/linter/rules/deprecated-functions.ts @@ -11,7 +11,7 @@ import type { LintingResult, LintingRule } from '../linter-format'; import { LintingPrettyPrintContext, LintingResultCertainty, LintingRuleCertainty } from '../linter-format'; import { LintingRuleTag } from '../linter-tags'; import { type FunctionsMetadata, functionFinderUtil } from './function-finder-util'; -import { parseRRange } from '../../util/r-version'; +import { RRange } from '../../util/r-version'; /** * Information about an argument of a function that should be flagged as deprecated if it is called with this argument @@ -71,7 +71,7 @@ export interface DeprecatedFunctionsConfig extends MergeableRecord{ const DeprecatedFunctions = [ - { name: 'geom_violin', whenArgs: [{ argName: 'draw_quantiles', state: DeprecationState.Deprecated, replacedBy: 'quantile.linetype', sinceVersion: parseRRange('>= 4.0.0') }] }, + { name: 'geom_violin', whenArgs: [{ argName: 'draw_quantiles', state: DeprecationState.Deprecated, replacedBy: 'quantile.linetype', sinceVersion: RRange.parse('>= 4.0.0') }] }, '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' ] satisfies (string | DeprecatedFunctionInformation)[]; diff --git a/test/functionality/linter/lint-deprecated-functions.test.ts b/test/functionality/linter/lint-deprecated-functions.test.ts index 52e59300442..106c56211ff 100644 --- a/test/functionality/linter/lint-deprecated-functions.test.ts +++ b/test/functionality/linter/lint-deprecated-functions.test.ts @@ -3,7 +3,7 @@ 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 { parseRRange } from '../../../src/util/r-version'; +import { RRange } from '../../../src/util/r-version'; describe('flowR linter', withTreeSitter(parser => { describe('deprecated functions', () => { @@ -85,11 +85,11 @@ dplyr::all_equal(first, second)`, 'deprecated-functions', replacedBy: 'foo', function: 'testFn', state: DeprecationState.Deprecated, - sinceVersion: parseRRange('>= 4.0.0'), + sinceVersion: RRange.parse('>= 4.0.0'), loc: [1, 8, 1, 13] }], { totalCalls: 1, totalFunctionDefinitions: 1 }, - { fns: [{ name: 'testFn', whenArgs: [{ argName: 'badArg', state: DeprecationState.Deprecated, replacedBy: 'foo', sinceVersion: parseRRange('>= 4.0.0') }] } ] } + { fns: [{ name: 'testFn', whenArgs: [{ argName: 'badArg', state: DeprecationState.Deprecated, replacedBy: 'foo', sinceVersion: RRange.parse('>= 4.0.0') }] } ] } ); }); }); From ae5965bc36c4900563cf8c664ea6221aadeb4cd3 Mon Sep 17 00:00:00 2001 From: gigalasr Date: Thu, 16 Jul 2026 13:38:33 +0200 Subject: [PATCH 7/7] docs(eval): describe interface and deprecation state - #1870 --- src/linter/rules/deprecated-functions.ts | 123 +++++++++++------- .../linter/lint-deprecated-functions.test.ts | 14 +- 2 files changed, 85 insertions(+), 52 deletions(-) diff --git a/src/linter/rules/deprecated-functions.ts b/src/linter/rules/deprecated-functions.ts index 1c6cbb861b3..39ade532288 100644 --- a/src/linter/rules/deprecated-functions.ts +++ b/src/linter/rules/deprecated-functions.ts @@ -1,5 +1,6 @@ import type { Range } from 'semver'; import type { BrandedIdentifier } from '../../dataflow/environments/identifier'; +import { Identifier } from '../../dataflow/environments/identifier'; import { FunctionArgument } from '../../dataflow/graph/graph'; import { isFunctionCallVertex } from '../../dataflow/graph/vertex'; import { EmptyArgument } from '../../r-bridge/lang-4.x/ast/model/nodes/r-function-call'; @@ -16,74 +17,103 @@ import { RRange } from '../../util/r-version'; /** * Information about an argument of a function that should be flagged as deprecated if it is called with this argument */ -export interface DeprecatedArgumentInformation { - argIdx?: number, - argName?: string - ifValue?: RegExp | string - replacedBy?: string - sinceVersion?: Range - state?: DeprecationState +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 */ -export interface DeprecatedFunctionInformation { - name: string - /** Used to mark specific arguments as deprecated */ - whenArgs?: DeprecatedArgumentInformation[] - replacedBy?: string - sinceVersion?: Range - state?: DeprecationState +interface DeprecatedFunctionInformation { + /** Mark specific arguments as deprecated */ + readonly whenArgs?: DeprecatedArgumentInformation[] + /** Suggested replacement for this function */ + readonly replacedBy?: string + /** The version since this function is deprecated */ + readonly sinceVersion?: Range + /** The state of deprecation {@link DeprecationState}, i.e. is the function completely removed, or are there better alternatives */ + readonly state?: DeprecationState } +/** + * Result of the {@link DEPRECATED_FUNCTIONS} linting rule + * See also the specializations {@link DeprecatedFunctionResult} and {@link DeprecatedArgumentResult} + */ export interface DeprecatedFunctionResultBase extends LintingResult { - function: BrandedIdentifier - replacedBy?: string - sinceVersion?: Range - state?: DeprecationState + /** 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 { - type: 'deprecated-function' + 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 { - type: 'deprecated-argument' - arg: string | number + readonly type: 'deprecated-argument' + /** The name or index of the deprecated argument */ + readonly arg: string | number } export type DeprecatedFunctionRuleResult = DeprecatedFunctionResult | DeprecatedArgumentResult; export enum DeprecationState { - /** Still works, but marked for removal */ + /** 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 marked for removal */ - Defunct = 'defunct', - /** Replaced by another function but kept for compatibility */ - Superseeded = 'superseeded' + /** 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 mark as deprecated */ - fns: (string | DeprecatedFunctionInformation)[] +export interface DeprecatedFunctionsConfig extends MergeableRecord { + /** Functions to always mark as deprecated */ + always: BrandedIdentifier[] + /** Functions to mark as deprecated for specific argument, argument value or version */ + conditionally: Record } - -const DeprecatedFunctions = [ - { name: 'geom_violin', whenArgs: [{ argName: 'draw_quantiles', state: DeprecationState.Deprecated, replacedBy: 'quantile.linetype', sinceVersion: RRange.parse('>= 4.0.0') }] }, +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' -] satisfies (string | DeprecatedFunctionInformation)[]; +]; + +const ConditionallyDeprecated = { + 'geom_violin': { whenArgs: [{ argName: 'draw_quantiles', state: DeprecationState.Deprecated, replacedBy: 'quantile.linetype', sinceVersion: RRange.parse('>= 4.0.0') }] }, +} satisfies Record; + +function getDeprecatedFunctionNames(config: DeprecatedFunctionsConfig): BrandedIdentifier[] { + return config.always.concat( + Object.entries(config.conditionally).map(([k, _]) => k) + ); +} export const DEPRECATED_FUNCTIONS = { - createSearch: (config) => functionFinderUtil.createSearch(config.fns.map(fn => typeof fn === 'string' ? fn : fn.name)), + createSearch: (config) => functionFinderUtil.createSearch(getDeprecatedFunctionNames(config)), processSearchResult: async(elements, config, data) => { - const fnInfosEx = Object.fromEntries( - config.fns - .filter(f => typeof f === 'object') - .map(f => [f.name, f]) - ); - const dataflow = (await data.dataflow()).graph; const idMap = (await data.normalize()).idMap; @@ -106,7 +136,7 @@ export const DEPRECATED_FUNCTIONS = { }).filter(p => isNotUndefined(p)); const results = detectedFunctions.map(e => { - const info = fnInfosEx[e.target]; + const info = config.conditionally[e.target]; if(info === undefined) { // No extra info => function is always marked as deprecated return [{ @@ -145,7 +175,7 @@ export const DEPRECATED_FUNCTIONS = { } // Don't mark function as deprecated, if we didn't find any deprecated args - }).filter(p => p !== undefined); + }).filter(p => isNotUndefined(p)); } else { // Extra Info but no whenArgs => always marked as deprecated and extra info is provided return [{ @@ -166,14 +196,14 @@ export const DEPRECATED_FUNCTIONS = { }; }, prettyPrint: { - [LintingPrettyPrintContext.Query]: (result: DeprecatedFunctionRuleResult) => `${result.type === 'deprecated-argument' ? `Argument \`${result.arg}\` of ` : ''}Function \`${result.function}\` at ${SourceLocation.format(result.loc)}`, + [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 \`${result.function}\` is ${result.state ?? 'deprecated'}`); + str.push(`Function \`${Identifier.toString(result.function)}\` is ${result.state ?? 'deprecated'}`); if(result.sinceVersion) { str.push(`since version ${result.sinceVersion.format()}`); } @@ -189,6 +219,9 @@ export const DEPRECATED_FUNCTIONS = { // ensures all deprecated functions found are actually deprecated through its limited config, but doesn't find all deprecated functions since the config is pre-crawled certainty: LintingRuleCertainty.BestEffort, description: 'Marks deprecated functions that should not be used anymore.', - defaultConfig: { fns: DeprecatedFunctions } + defaultConfig: { + always: AlwaysDeprecated, + conditionally: ConditionallyDeprecated + } } } as const satisfies LintingRule; diff --git a/test/functionality/linter/lint-deprecated-functions.test.ts b/test/functionality/linter/lint-deprecated-functions.test.ts index 106c56211ff..f2e8e59baf5 100644 --- a/test/functionality/linter/lint-deprecated-functions.test.ts +++ b/test/functionality/linter/lint-deprecated-functions.test.ts @@ -11,7 +11,7 @@ describe('flowR linter', withTreeSitter(parser => { assertLinter('no function listed', parser, 'cat("hello")\nprint("hello")\nx <- 1\ncat(x)', 'deprecated-functions', [], { totalCalls: 0, totalFunctionDefinitions: 0 }, - { fns: [] } + { 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)', @@ -20,7 +20,7 @@ describe('flowR linter', withTreeSitter(parser => { { certainty: LintingResultCertainty.Certain, function: 'cat', loc: [4, 1, 4, 6], type: 'deprecated-function' }, ], { totalCalls: 2, totalFunctionDefinitions: 2 }, - { fns: ['cat'] } + { 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)', @@ -28,7 +28,7 @@ describe('flowR linter', withTreeSitter(parser => { { certainty: LintingResultCertainty.Certain, function: 'cat', loc: [1, 1, 1, 12], type: 'deprecated-function' } ], { totalCalls: 1, totalFunctionDefinitions: 1 }, - { fns: ['cat'] } + { 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)', @@ -58,13 +58,13 @@ dplyr::all_equal(first, second)`, 'deprecated-functions', 'deprecated-functions', [{ certainty: LintingResultCertainty.Certain, function: 'recode', loc: [2, 1, 2, 9], type: 'deprecated-function' }], { totalCalls: 1, totalFunctionDefinitions: 1 }, - { fns: ['recode'], sigDb: controlledSigDb('dplyr', ['recode', 'filter']) } + { 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], type: 'deprecated-function' }], { totalCalls: 1, totalFunctionDefinitions: 1 }, - { fns: ['recode'], noSigDb: true } + { always: ['recode'], noSigDb: true } ); }); @@ -73,7 +73,7 @@ dplyr::all_equal(first, second)`, 'deprecated-functions', 'deprecated-functions', [], { totalCalls: 1, totalFunctionDefinitions: 1 }, - { fns: [{ name: 'testFn', whenArgs: [{ argName: 'badArg', state: DeprecationState.Deprecated }] } ] } + { always: [], conditionally: { 'testFn': { whenArgs: [{ argName: 'badArg', state: DeprecationState.Deprecated }] } } } ); assertLinter('deprecated arg present', parser, 'testFn(badArg=5)', @@ -89,7 +89,7 @@ dplyr::all_equal(first, second)`, 'deprecated-functions', loc: [1, 8, 1, 13] }], { totalCalls: 1, totalFunctionDefinitions: 1 }, - { fns: [{ name: 'testFn', whenArgs: [{ argName: 'badArg', state: DeprecationState.Deprecated, replacedBy: 'foo', sinceVersion: RRange.parse('>= 4.0.0') }] } ] } + { always: [], conditionally: { 'testFn': { whenArgs: [{ argName: 'badArg', state: DeprecationState.Deprecated, replacedBy: 'foo', sinceVersion: RRange.parse('>= 4.0.0') }] } } } ); }); });