Skip to content

rule: property visibility mismatch#100

Merged
runem merged 6 commits intorunem:1.2.0from
43081j:visibility-mismatch
May 18, 2020
Merged

rule: property visibility mismatch#100
runem merged 6 commits intorunem:1.2.0from
43081j:visibility-mismatch

Conversation

@43081j
Copy link
Contributor

@43081j 43081j commented May 9, 2020

this tries to do something along the lines of what #79 wanted i guess.

this bit is questionable:

	return member.kind === 'property' && member.meta?.attribute === false;

to detect if @internalProperty was used. it is upto you if you want to introduce something in WCA so component members have an explicit flag when they use that decorator. this should work fine though since making a property not use attributes is close enough to being internal anyway.

@43081j 43081j changed the base branch from master to 1.2.0 May 9, 2020 18:29
@runem
Copy link
Owner

runem commented May 11, 2020

You should be able to access the decorator through member.meta which is of the following type:

interface LitElementPropertyConfig {
    type?: SimpleType | string;
    attribute?: string | boolean;
    node?: {
        type?: Node;
        attribute?: Node;
        decorator?: CallExpression;
    };
    hasConverter?: boolean;
    default?: unknown;
    reflect?: boolean;
}

So you would be able to check for member.meta?.node?.decorator?.identifier.text to get the name of the decorator 👍

@43081j
Copy link
Contributor Author

43081j commented May 11, 2020

i updated it, didn't realise you could access the typescript node there.

remember this is based on 1.2.0 too FYI

edit: it works, but i used member.node rather than member.meta.node.decorator. is there any particular reason you have both? this way we can tolerate multiple decorators and still match...

@43081j 43081j force-pushed the visibility-mismatch branch 2 times, most recently from cee845f to 91567b6 Compare May 11, 2020 17:40
@43081j 43081j force-pushed the visibility-mismatch branch from 91567b6 to 66fd93f Compare May 11, 2020 18:13
@runem
Copy link
Owner

runem commented May 12, 2020

member.node is the node where the property/attribute was found/emitted. This could be a PropertyDeclaration, ExpressionStatement, PropertyAssignment, eg. depending on how it was found.

member.meta contains specific metadata of the analysis of the property/attribute. This data-structure right now only contains lit-specific information, but I plan that this data-structure in the future could depend on the specific WCA-flavor that emitted it. For example, if there was to be created a Stencil-specific WCA-flavor, the meta-field would contain Stencil-specific data.

member.meta.node.decorator will always point to the decorator that contains LitElement-specific metadata which was found during the analysis of the PropertyDeclaration. The decorator would always be either a property- or internalProperty-CallExpression and other decorators would be ignored. If there are multiple decorators, this would still be either the property or internalProperty decorator. The loop over member.node.decorators has already been done in WCA and the decorator of interrest would be assigned to member.meta.node.decorator, so you can trust the text property of that node to be internalProperty when that's the case.

@43081j 43081j force-pushed the visibility-mismatch branch from fcdd20a to cf0f1ca Compare May 12, 2020 15:30
@43081j
Copy link
Contributor Author

43081j commented May 12, 2020

i updated it. just wasn't aware what the meta node was so thanks for the explanation. i do wonder what'll happen when one day multiple decorators might become a thing in lit, though. maybe it should be an array rather than assuming there will always be just one "useful one" per property.

@runem runem merged commit dd0b22c into runem:1.2.0 May 18, 2020
@43081j 43081j deleted the visibility-mismatch branch May 18, 2020 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants