Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
282 changes: 257 additions & 25 deletions src/linter/rules/deprecated-functions.ts
Original file line number Diff line number Diff line change
@@ -1,34 +1,176 @@
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 { isFunctionCallVertex, 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 { testFunctionsIgnoringPackage } from '../../search/flowr-search-filters';
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 { type FunctionsMetadata } from './function-finder-util';
import { RRange } from '../../util/r-version';
import { Q } from '../../search/flowr-search-builder';
import { testFunctionsIgnoringPackage } from '../../search/flowr-search-filters';
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';

/**
* Information about an argument of a function that should be flagged as deprecated if it is called with this argument
*/
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
*/
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
}

/**
* Result of the {@link DEPRECATED_FUNCTIONS} linting rule
* See also the specializations {@link DeprecatedFunctionResult} and {@link DeprecatedArgumentResult}
*/
export interface DeprecatedFunctionResultBase extends LintingResult {
Comment thread
gigalasr marked this conversation as resolved.
/** 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',
Comment thread
gigalasr marked this conversation as resolved.
/** 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: BrandedIdentifier[]
/** Functions to mark as deprecated for specific argument, argument value or version */
conditionally: Record<BrandedIdentifier, DeprecatedFunctionInformation>
}

interface PotentialFunction {
node: RNode<ParentInformation>;
target: BrandedIdentifier;
sourceLocation: SourceLocation
}

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': { whenArgs: [{ argName: 'draw_quantiles', state: DeprecationState.Deprecated, replacedBy: 'quantile.linetype', sinceVersion: RRange.parse('>= 4.0.0') }] },
} satisfies Record<BrandedIdentifier, DeprecatedFunctionInformation>;

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, { onlyBuiltin: true }),
createSearch: (_config) => Q.all().filter(VertexType.FunctionCall).with(Enrichment.CallTargets, { onlyBuiltin: true }),
processSearchResult: async(elements, config, data) => {
const matchesConfiguredFns = testFunctionsIgnoringPackage(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 = testFunctionsIgnoringPackage(config.always);
const graph = (await data.dataflow()).graph;
const idMap = (await data.normalize()).idMap;

const metadata: FunctionsMetadata = {
totalCalls: 0,
totalFunctionDefinitions: 0
};

// 1. Collect all function call targets from detected function calls
const detectedFunctions = elements.getElements().flatMap(e => {
metadata.totalCalls++;
return enrichmentContent(e, Enrichment.CallTargets).targets.map(target => {
metadata.totalFunctionDefinitions++;
const sourceLocation = SourceLocation.fromNode(e.node);
if(sourceLocation !== undefined) {
return {
node: e.node, target: target as BrandedIdentifier, sourceLocation
};
}
});
}).filter(p => isNotUndefined(p));

// 2. Uses hardcoded information about deprecated arguments and deprecated functions
const results: DeprecatedFunctionRuleResult[] = detectedFunctions.map(candidate => {
const info = config.conditionally[candidate.target];
if(isNotUndefined(info)) {
// Check functions from DeprecatedFunctionsConfig.conditionally
return deprecateFunctionConditionally(candidate, graph, idMap, 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': metadata };
}

// 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)) {
Expand All @@ -48,6 +190,7 @@
}
alreadyFlagged.add(id);
sigdbFlagged.push({
type: 'deprecated-function',
certainty: LintingResultCertainty.Certain,
involvedId: id,
function: Identifier.toString(qualified),
Expand All @@ -56,23 +199,112 @@
}

return {
results: [...hardcoded.results, ...sigdbFlagged],
'.meta': {
totalCalls: hardcoded['.meta'].totalCalls + sigdbFlagged.length,
totalFunctionDefinitions: hardcoded['.meta'].totalFunctionDefinitions + sigdbFlagged.length
}
results: results.concat(sigdbFlagged),
'.meta': metadata
};
},
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()}`);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

shouldn't we check whether the since Version range contract is even fulfilled?

}
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<FunctionsResult, FunctionsMetadata, FunctionsToDetectConfig>;
} as const satisfies LintingRule<DeprecatedFunctionRuleResult, FunctionsMetadata, DeprecatedFunctionsConfig>;

/**
* This function is applied to function candidates that have an entry in the {@link DeprecatedFunctionsConfig.conditionally} map.
*/
function deprecateFunctionConditionally(candidate: PotentialFunction, dataflow: DataflowGraph, idMap: AstIdMap, info: DeprecatedFunctionInformation): DeprecatedFunctionRuleResult[] {
const results: DeprecatedFunctionRuleResult[] = [];

// If when args is provided, mark the function as deprecated (and its argument) if the respective argument is present
if(info.whenArgs) {
for(const deprecatedArgInfo of info.whenArgs) {
const vertex = dataflow.getVertex(candidate.node.info.id);
if(vertex === undefined || !isFunctionCallVertex(vertex)) {
continue;
}

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;
}

results.push({
type: 'deprecated-argument',
certainty: LintingResultCertainty.Certain,
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);
}
}

// Otherwise, check version and deprecate entire function in case of version range match
if(info.sinceVersion) {
// TODO: Version Check

Check failure on line 278 in src/linter/rules/deprecated-functions.ts

View workflow job for this annotation

GitHub Actions / 👩‍🏫 Linting on Main

Unexpected 'todo' comment: 'TODO: Version Check'
results.push({
type: 'deprecated-function',
certainty: 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(candidate.target)) {
return undefined;
}

return {
type: 'deprecated-function',
certainty: LintingResultCertainty.Certain,
involvedId: candidate.node.info.id,
loc: candidate.sourceLocation,
function: candidate.target,
} satisfies DeprecatedFunctionResult;
}
2 changes: 1 addition & 1 deletion src/linter/rules/naming-convention.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}

Expand Down
8 changes: 3 additions & 5 deletions src/linter/rules/network-functions.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,15 @@
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';
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
}
Expand Down
Loading
Loading