Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

fix(rome_js_analyze): fix noShoutyConstants with empty string #3868

Merged
merged 6 commits into from
Nov 28, 2022
Merged

fix(rome_js_analyze): fix noShoutyConstants with empty string #3868

merged 6 commits into from
Nov 28, 2022

Conversation

rosslh
Copy link
Contributor

@rosslh rosslh commented Nov 26, 2022

Summary

Closes #3867

Test Plan

cargo test -p rome_js_analyze -- shouty

With some new test cases which were missing:

const Empty = "";
export const E = Empty;

const NotMatching = "DoesNotMatch";
export const F = NotMatching;

const matchingLowercase = "matchingLowercase";
export const G = matchingLowercase;

const AL = "ALPHA";
export const H = AL;

const ALPHA = "AL";
export const I = ALPHA;

@netlify
Copy link

netlify bot commented Nov 26, 2022

Deploy Preview for docs-rometools canceled.

Built without sensitive environment variables

Name Link
🔨 Latest commit 3218215
🔍 Latest deploy log https://app.netlify.com/sites/docs-rometools/deploys/638269adfe7bb40008800b29

if id_text.len() != usize::from(literal_text.len()) {
return None;
}

for (from_id, from_literal) in id_text.chars().zip(literal_text.chars()) {
if from_id != from_literal {
return None;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: We can remove one of the is_lowercase checks below. Either they are both lowercase or both are not.

@MichaReiser
Copy link
Contributor

Thank you!

@MichaReiser MichaReiser merged commit 20e2ed0 into rome:main Nov 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 false positive for noShoutyConstants on empty strings
2 participants