Skip to content
Open
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
4 changes: 3 additions & 1 deletion src/linter/linter-rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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;
Expand Down
176 changes: 176 additions & 0 deletions src/linter/rules/unclosed-connection.ts
Original file line number Diff line number Diff line change
@@ -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<NodeId, NodeId> = 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<NodeId, NodeId>());
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<NodeId, NodeId>());

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<UnclosedConnectionResult>[],
'.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<UnclosedConnectionResult, UnclosedConnectionMetadata, UnclosedConnectionConfig>;
Original file line number Diff line number Diff line change
@@ -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;
Original file line number Diff line number Diff line change
@@ -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;
113 changes: 113 additions & 0 deletions test/functionality/linter/lint-unclosed-connection.test.ts
Original file line number Diff line number Diff line change
@@ -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]
}]
);
});
}));
Loading