Skip to content

Commit

Permalink
Fix #124 unused variable actually used in describe method (#235)
Browse files Browse the repository at this point in the history
* Fix #124 unused variable actually used in describe method

* First  version: removing debugging code

* Fix #124

* Linter fixes

* Fixing TS complain

* Enhancements according to Nahue review
  • Loading branch information
fdodino authored Apr 19, 2024
1 parent 640709e commit a5427d7
Showing 1 changed file with 27 additions and 28 deletions.
55 changes: 27 additions & 28 deletions src/helpers.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { BOOLEAN_MODULE, CLOSURE_EVALUATE_METHOD, CLOSURE_TO_STRING_METHOD, INITIALIZE_METHOD, KEYWORDS, NUMBER_MODULE, OBJECT_MODULE, STRING_MODULE, WOLLOK_BASE_PACKAGE } from './constants'
import { List, count, is, isEmpty, last, match, notEmpty, when } from './extensions'
import { Assignment, Body, Class, Describe, Entity, Environment, Expression, Field, If, Import, Literal, LiteralValue, Method, Module, NamedArgument, New, Node, Package, Parameter, ParameterizedType, Program, Reference, Return, Self, Send, Sentence, Singleton, Super, Test, Throw, Try, Variable } from './model'
import { Assignment, Body, Class, Entity, Environment, Expression, Field, If, Import, Literal, LiteralValue, Method, Module, NamedArgument, New, Node, Package, Parameter, ParameterizedType, Program, Reference, Return, Self, Send, Sentence, Singleton, Super, Test, Throw, Try, Variable } from './model'

export const LIBRARY_PACKAGES = ['wollok.lang', 'wollok.lib', 'wollok.game', 'wollok.vm', 'wollok.mirror']

Expand Down Expand Up @@ -146,29 +146,6 @@ export const isCallToSuper = (node: Node): boolean =>

export const isGetter = (node: Method): boolean => node.parent.allFields.map(_ => _.name).includes(node.name) && isEmpty(node.parameters)

export const methodOrTestUsesField = (parent: Method | Test, field: Field): boolean => parent.sentences.some(sentence => usesField(sentence, field))

export const usesField = (node: Sentence | Body | NamedArgument, field: Field): boolean =>
match(node)(
when(Singleton)(node => {
if (!node.isClosure()) return false
const applyMethod = node.methods.find(method => method.name === CLOSURE_EVALUATE_METHOD)
return !!applyMethod && methodOrTestUsesField(applyMethod, field)
}),
when(Variable)(node => usesField(node.value, field)),
when(Return)(node => !!node.value && usesField(node.value, field)),
when(Assignment)(node => node.variable.target === field || usesField(node.value, field)),
when(Reference)(node => node.target === field),
when(Send)(node => usesField(node.receiver, field) || node.args.some(arg => usesField(arg, field))),
when(If)(node => usesField(node.condition, field) || usesField(node.thenBody, field) || node.elseBody && usesField(node.elseBody, field)),
when(New)(node => node.args.some(arg => usesField(arg, field))),
when(NamedArgument)(node => usesField(node.value, field)),
when(Throw)(node => usesField(node.exception, field)),
when(Try)(node => usesField(node.body, field) || node.catches.some(catchBlock => usesField(catchBlock.body, field)) || !!node.always && usesField(node.always, field)),
when(Expression)(() => false),
when(Body)(node => node.sentences.some(sentence => usesField(sentence, field))),
)

// TODO: Import could offer a list of imported entities
export const entityIsAlreadyUsedInImport = (target: Entity | undefined, entityName: string): boolean | undefined => target && match(target)(
when(Package)(node => node.members.some(member => member.name == entityName)),
Expand Down Expand Up @@ -205,13 +182,35 @@ export const assignsVariable = (sentence: Sentence | Body, variable: Variable |

export const unusedVariable = (node: Field): boolean => {
const parent = node.parent
const allFields = parent.allFields
const allMethods: List<Test | Method> = parent.is(Describe) ? parent.tests : parent.allMethods
return !node.isProperty && node.name != CLOSURE_TO_STRING_METHOD
&& allMethods.every(method => !methodOrTestUsesField(method, node))
&& allFields.every(field => !usesField(field.value, node))
&& parent.allMembers.every((member: Field | Method | Variable | Test) => !usesField(member, node))
}

export const usesField = (node: Node, field: Field): boolean =>
match(node)(
when(Singleton)(node => {
if (!node.isClosure()) return false
const applyMethod = node.methods.find(method => method.name === CLOSURE_EVALUATE_METHOD)
return !!applyMethod && usesField(applyMethod, field)
}),
when(Variable)(node => usesField(node.value, field)),
when(Return)(node => !!node.value && usesField(node.value, field)),
when(Assignment)(node => node.variable.target === field || usesField(node.value, field)),
when(Reference)(node => node.target === field || !!node.target && node.target.is(Field) && usesField(node.target, field)),
when(Field)(node => node.value && (node.value.is(Literal) || node.value.is(Send)) && usesField(node.value, field)),
when(Literal)(node =>
// See type LiteralValue for collection values
Array.isArray(node.value) && node.value[1].some((expression: any) => usesField(expression, field))),
when(Send)(node => usesField(node.receiver, field) || node.args.some(arg => usesField(arg, field))),
when(If)(node => usesField(node.condition, field) || usesField(node.thenBody, field) || node.elseBody && usesField(node.elseBody, field)),
when(New)(node => node.args.some(arg => usesField(arg, field))),
when(NamedArgument)(node => usesField(node.value, field)),
when(Throw)(node => usesField(node.exception, field)),
when(Try)(node => usesField(node.body, field) || node.catches.some(catchBlock => usesField(catchBlock.body, field)) || !!node.always && usesField(node.always, field)),
when(Expression)(() => false),
when(Node)(node => (node.is(Body) || node.is(Method) || node.is(Test)) && node.sentences.some(sentence => usesField(sentence, field))),
)

export const usesReservedWords = (node: Class | Singleton | Variable | Field | Parameter): boolean => {
const parent = node.ancestors.find(ancestor => ancestor.is(Package)) as Package | undefined
const wordsReserved = LIBRARY_PACKAGES.flatMap(libPackage => node.environment.getNodeByFQN<Package>(libPackage).members.map(_ => _.name))
Expand Down

0 comments on commit a5427d7

Please sign in to comment.