diff --git a/src/linter/linter-rules.ts b/src/linter/linter-rules.ts index 93226c91798..55dc34fa21e 100644 --- a/src/linter/linter-rules.ts +++ b/src/linter/linter-rules.ts @@ -17,6 +17,7 @@ import { SOFTWARE_HAS_TESTS } from './rules/software-has-tests'; import { NO_LEAKED_CREDENTIALS } from './rules/no-leaked-credentials'; import { UNDEFINED_SYMBOL } from './rules/undefined-symbol'; import { SYNTACTICALLY_VALID } from './rules/syntactically-valid'; +import { UNCLOSED_CONNECTION } from './rules/unclosed-connection'; /** * The registry of currently supported linting rules. @@ -40,7 +41,8 @@ export const LintingRules = { 'software-has-tests': SOFTWARE_HAS_TESTS, 'no-leaked-credentials': NO_LEAKED_CREDENTIALS, 'undefined-symbol': UNDEFINED_SYMBOL, - 'syntactically-valid': SYNTACTICALLY_VALID + 'syntactically-valid': SYNTACTICALLY_VALID, + 'unclosed-connection': UNCLOSED_CONNECTION } as const; export type LintingRuleNames = keyof typeof LintingRules; diff --git a/src/linter/rules/unclosed-connection.ts b/src/linter/rules/unclosed-connection.ts new file mode 100644 index 00000000000..87caa3653c6 --- /dev/null +++ b/src/linter/rules/unclosed-connection.ts @@ -0,0 +1,176 @@ +import type { Writable } from 'ts-essentials'; +import type { DataflowGraphVertexFunctionCall } from '../../dataflow/graph/vertex'; +import { Q } from '../../search/flowr-search-builder'; +import { isNotUndefined, isUndefined } from '../../util/assert'; +import type { MergeableRecord } from '../../util/objects'; +import { SourceLocation } from '../../util/range'; +import { LintingPrettyPrintContext, LintingResultCertainty, LintingRuleCertainty } from '../linter-format'; +import type { LintingResult, LintingRule } from '../linter-format'; +import { LintingRuleTag } from '../linter-tags'; +import { pMatch } from '../../dataflow/internal/linker'; +import { EdgeType } from '../../dataflow/graph/edge'; +import type { NodeId } from '../../r-bridge/lang-4.x/ast/model/processing/node-id'; +import { dataflowLogger } from '../../dataflow/logger'; +import { Enrichment } from '../../search/search-executor/search-enrichers'; +import { getOriginInDfg, OriginType } from '../../dataflow/origin/dfg-get-origin'; +import { OpenConnectionFunctions } from '../../queries/catalog/dependencies-query/function-info/open-connection-functions'; +import { CloseConnectionFunctions } from '../../queries/catalog/dependencies-query/function-info/close-connection-functions'; +import type { CallContextQueryResult, CallContextQuerySubKindResult } from '../../queries/catalog/call-context-query/call-context-query-format'; + +export type UnclosedConnectionResult = LintingResult; + +export type UnclosedConnectionConfig = MergeableRecord; + +export type UnclosedConnectionMetadata = MergeableRecord; + + +export const UNCLOSED_CONNECTION = { + createSearch: () => Q.fromQuery([ { + 'type': 'call-context', + 'callName': `^${OpenConnectionFunctions.map(element => { + return element.name; + }).join('|')}$`, + 'kind': 'connection', + 'subkind': 'openConnection' + }, + { + 'type': 'call-context', + 'callName': `^${CloseConnectionFunctions.map(element => { + return element.name; + }).join('|')}$`, + 'kind': 'connection', + 'subkind': 'closeConnection' + }]), + processSearchResult: async(elements, _config, data) => { + const dataflow = await data.dataflow(); + const dependencies = (((elements.enrichmentContent(Enrichment.QueryData).queries as { 'call-context': CallContextQueryResult })['call-context'].kinds) as { connection: { subkinds: { openConnection: CallContextQuerySubKindResult[], closeConnection: CallContextQuerySubKindResult[] } } }).connection.subkinds; + //Map: [NodeId of open-call, NodeId of the variable that it is defined by] + const builtInOpen = dependencies.openConnection.filter(element => { + const origins = getOriginInDfg(dataflow.graph, element.id); + if(isNotUndefined(origins)) { + const builtIn = origins.every(e => e.type === OriginType.BuiltInFunctionOrigin); + if(!builtIn){ + return false; + } + } + return true; + }); + const openedByDefiningVar: Map = builtInOpen.map(element => { + const h = dataflow.graph.ingoingEdges(element.id); + //case: open() instead of a <- open() + if(isUndefined(h)){ + return undefined; + } + for(const [toNode, edge] of h){ + if(edge.types === EdgeType.DefinedBy){ + return [element.id, toNode]; + } + } + }).filter(element => { + return isNotUndefined(element); + }).reduce((map, [definer, openNode]) => { + map.set(definer, openNode); + return map; + }, new Map()); + const openCalls = new Set(builtInOpen.map(element => { + return element.id; + })); + //Map: [ NodeId of defining symbol that it closes, NodeId of close-call] + const closedArg = dependencies.closeConnection.filter(element => { + const origins = getOriginInDfg(dataflow.graph, element.id); + if(isNotUndefined(origins)) { + const builtIn = origins.every(e => e.type === OriginType.BuiltInFunctionOrigin); + if(!builtIn){ + return false; + } + } + return true; + }).map(element => { + return dataflow.graph.getVertex(element.id) as DataflowGraphVertexFunctionCall; + }).map(element => { + const closeParamMap = { + '...': '...' + } as const; + const mapping = pMatch(element.args, closeParamMap); + const mappedToStop = mapping.get('...'); + if(isUndefined(mappedToStop) || mappedToStop.length === 0 || isUndefined(mappedToStop[0])){ + dataflowLogger.warn(`Argument of call with id ${element.id} could not be resolved`); + return undefined; + } + const oldArgId = mappedToStop[0]; + const box = [oldArgId]; + //search for an openCall that might get closed by this call using the defining variable of the openCall + while(box.length > 0){ + const id = box.pop() as NodeId; + const h = dataflow.graph.outgoingEdges(id); + if(isUndefined(h)){ + break; + } + for(const [toNode, edge] of h){ + if(edge.types === EdgeType.Reads || edge.types === EdgeType.DefinedBy){ + box.push(toNode); + } + if(edge.types === EdgeType.DefinedBy && openedByDefiningVar.has(toNode)){ + return [id, element.id]; + } + } + } + return undefined; + }).filter(element => { + return isNotUndefined(element); + }) + .reduce((map, [node, arg]) => { + map.set(node, arg); + return map; + }, new Map()); + + return { + results: + elements.getElements() + //filter out close-calls + .filter(element => { + return openCalls.has(element.node.info.id); + }) + .filter(element => { + if(openedByDefiningVar.has(element.node.info.id)){ + const ident = openedByDefiningVar.get(element.node.info.id); + //closed by one call + if(isNotUndefined(ident) && closedArg.has(ident)){ + const closeCall = closedArg.get(ident) as NodeId; + const closeCallDependencies = new Set(dataflow.graph.getVertex(closeCall)?.cds); + const openCallDependencies = new Set(dataflow.graph.getVertex(element.node.info.id)?.cds); + //both or neither call is executed + if(openCallDependencies.isSubsetOf(closeCallDependencies) && closeCallDependencies.isSubsetOf(openCallDependencies)){ + return false; + } else { + return true; + } + } else { + return true; + } + //can't be closed because of form: open(), instead of: a <- open() + } else { + return true; + } + }) + .map(element => ({ + certainty: LintingResultCertainty.Uncertain, + involvedId: element.node.info.id, + loc: SourceLocation.fromNode(element.node) + })) + .filter(element => isNotUndefined(element.loc)) as Writable[], + '.meta': {} + }; + }, + prettyPrint: { + [LintingPrettyPrintContext.Query]: result => `Open connection at ${SourceLocation.format(result.loc)} might not get closed.`, + [LintingPrettyPrintContext.Full]: result => `Open connection at ${SourceLocation.format(result.loc)} might not get closed.` + }, + info: { + name: 'Unclosed Connection', + tags: [LintingRuleTag.Robustness, LintingRuleTag.Smell], + certainty: LintingRuleCertainty.BestEffort, + description: 'Flags calls which open a connection that is (not necessarily) closed.', + defaultConfig: {} + } +} as const satisfies LintingRule; \ No newline at end of file diff --git a/src/queries/catalog/dependencies-query/function-info/close-connection-functions.ts b/src/queries/catalog/dependencies-query/function-info/close-connection-functions.ts new file mode 100644 index 00000000000..16f8145c6b2 --- /dev/null +++ b/src/queries/catalog/dependencies-query/function-info/close-connection-functions.ts @@ -0,0 +1,5 @@ +import type { FunctionInfo } from './function-info'; + +export const CloseConnectionFunctions: FunctionInfo[] = [ + { package: 'base', name: 'close', argIdx: 0, argName: 'package', resolveValue: true } +] as const; \ No newline at end of file diff --git a/src/queries/catalog/dependencies-query/function-info/open-connection-functions.ts b/src/queries/catalog/dependencies-query/function-info/open-connection-functions.ts new file mode 100644 index 00000000000..46bce484198 --- /dev/null +++ b/src/queries/catalog/dependencies-query/function-info/open-connection-functions.ts @@ -0,0 +1,13 @@ +import type { FunctionInfo } from './function-info'; + +export const OpenConnectionFunctions: FunctionInfo[] = [ + { package: 'base', name: 'textConnection', argIdx: 0, argName: 'package', resolveValue: true }, + { package: 'base', name: 'open', argIdx: 0, argName: 'package', resolveValue: true }, + { package: 'base', name: 'file', argIdx: 0, argName: 'package', resolveValue: true }, + { package: 'base', name: 'url', argIdx: 0, argName: 'package', resolveValue: true }, + { package: 'base', name: 'gzfile', argIdx: 0, argName: 'package', resolveValue: true }, + { package: 'base', name: 'bzfile', argIdx: 0, argName: 'package', resolveValue: true }, + { package: 'base', name: 'xzfile', argIdx: 0, argName: 'package', resolveValue: true }, + { package: 'base', name: 'file', argIdx: 0, argName: 'package', resolveValue: true }, + { package: 'base', name: 'unz', argIdx: 0, argName: 'package', resolveValue: true }, +] as const; \ No newline at end of file diff --git a/test/functionality/linter/lint-unclosed-connection.test.ts b/test/functionality/linter/lint-unclosed-connection.test.ts new file mode 100644 index 00000000000..f8406620500 --- /dev/null +++ b/test/functionality/linter/lint-unclosed-connection.test.ts @@ -0,0 +1,113 @@ +import { describe } from 'vitest'; +import { withTreeSitter } from '../_helper/shell'; +import { assertLinter } from '../_helper/linter'; +import { LintingResultCertainty } from '../../../src/linter/linter-format'; + +describe('flowR linter', withTreeSitter(parser => { + describe('unclosed-connection', () => { + assertLinter('All closed', parser, `a <- textConnection(A) +readLines(a, 2) +file <- file() +b <- textConnection(B) + +close(a) +close(b) +close(file)`, + 'unclosed-connection', + [] + ); + assertLinter('Only one closed', parser, `a <- textConnection(AB) +b <- a +if(x){ + b <- textConnection(LETTERS) + close(b) + close(b) +} +t <- 2`, + 'unclosed-connection', + [{ + certainty: LintingResultCertainty.Uncertain, + loc: [1, 6, 1, 23] + }] + ); + assertLinter('Closed with new definer', parser, `a <- textConnection(AB) +b <- a +c <- b +close(c)`, + 'unclosed-connection', + [] + ); + assertLinter('Not necessarily closed', parser, `a <- textConnection(AB) +b <- textConnection(E) +if(x){ + close(a) +} +t <- 2 +close(b)`, + 'unclosed-connection', + [{ + certainty: LintingResultCertainty.Uncertain, + loc: [1, 6, 1, 23] + }] + ); + assertLinter('Openend and closed in different branches', parser, `a <- 4+3 +if(x){ + a <- textConnection(A) + b <- textConnection(B) +} +t <- 34 +if(x){ + close(a) +} +if(y){ + close(b) +}`, + 'unclosed-connection', + [{ + certainty: LintingResultCertainty.Uncertain, + loc: [3, 7, 3, 23] + }, + { + certainty: LintingResultCertainty.Uncertain, + loc: [4, 7, 4, 23] + }] + ); + assertLinter('Nested branches - not necessarily closed', parser, `a <- 4+3 +if(x){ + a <- textConnection(A) + b <- textConnection(B) + if(y){ + close(a) + } +}`, + 'unclosed-connection', + [{ + certainty: LintingResultCertainty.Uncertain, + loc: [3, 7, 3, 23] + }, + { + certainty: LintingResultCertainty.Uncertain, + loc: [4, 7, 4, 23] + }] + ); + assertLinter('Nested branches - not closed', parser, `if(x){ + a <- 4 + while(a > 0){ + b <- textConnection(A) + readLines(b, 2) + a <- a - 1 + } + close(b) +} +else { + a <- textConnection(A) + close(a) +}`, + 'unclosed-connection', + [{ + certainty: LintingResultCertainty.Uncertain, + loc: [4, 8, 4, 24] + }] + ); + }); +}));