Skip to content

Commit

Permalink
refactor(util): avoid repeated uri_base_name calls
Browse files Browse the repository at this point in the history
When classifying URIs to see if they are .d.ts files or .tsx files, we
effectively call uri_base_name multiple times. This is ugly.

Combine classification into one step. This is a nicer interface in my
opinion and it avoids the duplicate uri_base_name issue.

Unfortunately, this change adds another inefficiency: we search for
.tsx even when it's unnecessary. I think this inefficiency is worth
the improved interface.
  • Loading branch information
strager committed Oct 19, 2023
1 parent ae0bc6e commit ad6ee67
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 18 deletions.
2 changes: 2 additions & 0 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,8 @@ quick_lint_js_add_library(
quick-lint-js/util/ascii.h
quick-lint-js/util/binary-reader.h
quick-lint-js/util/binary-writer.h
quick-lint-js/util/classify-path.cpp
quick-lint-js/util/classify-path.h
quick-lint-js/util/cpp.h
quick-lint-js/util/instance-tracker.h
quick-lint-js/util/math-overflow.h
Expand Down
7 changes: 4 additions & 3 deletions src/quick-lint-js/lsp/lsp-language.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
#include <quick-lint-js/fe/linter.h>
#include <quick-lint-js/port/char8.h>
#include <quick-lint-js/util/algorithm.h>
#include <quick-lint-js/util/uri.h>
#include <quick-lint-js/util/classify-path.h>
#include <string_view>

namespace quick_lint_js {
Expand Down Expand Up @@ -84,10 +84,11 @@ struct LSP_Language {
return nullptr;
}
if (lang->typescript_autodetect) {
if (uri_looks_like_typescript_definition(uri)) {
Path_Classification classified_uri = classify_uri(uri);
if (classified_uri.typescript_definition) {
return &languages[5];
}
if (uri_looks_like_typescript_jsx(uri)) {
if (classified_uri.typescript_jsx) {
return &languages[8];
}
}
Expand Down
36 changes: 36 additions & 0 deletions src/quick-lint-js/util/classify-path.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// Copyright (C) 2020 Matthew "strager" Glazar
// See end of file for extended copyright information.

#include <cstddef>
#include <quick-lint-js/port/char8.h>
#include <quick-lint-js/util/classify-path.h>
#include <quick-lint-js/util/uri.h>

namespace quick_lint_js {
Path_Classification classify_uri(String8_View uri) {
// FIXME(strager): Should this unescape % encoding?
String8_View base_name = uri_base_name(uri);
return Path_Classification{
.typescript_definition = base_name.find(u8".d."_sv) != base_name.npos,
.typescript_jsx = base_name.find(u8".tsx"_sv) != base_name.npos,
};
}
}

// quick-lint-js finds bugs in JavaScript programs.
// Copyright (C) 2020 Matthew "strager" Glazar
//
// This file is part of quick-lint-js.
//
// quick-lint-js is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// quick-lint-js is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with quick-lint-js. If not, see <https://www.gnu.org/licenses/>.
43 changes: 43 additions & 0 deletions src/quick-lint-js/util/classify-path.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// Copyright (C) 2020 Matthew "strager" Glazar
// See end of file for extended copyright information.

#pragma once

#include <quick-lint-js/port/char8.h>

namespace quick_lint_js {
struct Path_Classification {
// True if the path has '.d.' in the base name.
//
// This tries to emulate the logic of TypeScript's isDeclarationFileName
// function [1]. However, this does not require ".ts". It will be true for an
// URI such as u8"file:///test.d.js".
//
// [1]
// https://github.com/microsoft/TypeScript/blob/daa7e985f5adc972aa241e5b0761c7dc433e94bf/src/compiler/parser.ts#L10408
bool typescript_definition;

// True if the path's base name ends with '.tsx'.
bool typescript_jsx;
};

Path_Classification classify_uri(String8_View uri);
}

// quick-lint-js finds bugs in JavaScript programs.
// Copyright (C) 2020 Matthew "strager" Glazar
//
// This file is part of quick-lint-js.
//
// quick-lint-js is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// quick-lint-js is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with quick-lint-js. If not, see <https://www.gnu.org/licenses/>.
13 changes: 0 additions & 13 deletions src/quick-lint-js/util/uri.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,6 @@ namespace quick_lint_js {
//
// FIXME(strager): This function is not robust at all.
String8_View uri_base_name(String8_View uri);

// Returns true if the URI has '.d.' in the base name.
//
// This tries to emulate the logic of TypeScript's isDeclarationFileName
// function [1]. However, this function does not require ".ts". It will return
// true for an URI such as u8"file:///test.d.js".
//
// [1]
// https://github.com/microsoft/TypeScript/blob/daa7e985f5adc972aa241e5b0761c7dc433e94bf/src/compiler/parser.ts#L10408
bool uri_looks_like_typescript_definition(String8_View uri);

// Returns true if the URI's base name ends with '.tsx'.
bool uri_looks_like_typescript_jsx(String8_View uri);
}

// quick-lint-js finds bugs in JavaScript programs.
Expand Down
5 changes: 3 additions & 2 deletions src/quick-lint-js/vscode/vscode-language.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
#include <quick-lint-js/fe/linter.h>
#include <quick-lint-js/port/char8.h>
#include <quick-lint-js/util/algorithm.h>
#include <quick-lint-js/util/uri.h>
#include <quick-lint-js/util/classify-path.h>
#include <string_view>

namespace quick_lint_js {
Expand Down Expand Up @@ -73,7 +73,8 @@ struct VSCode_Language {
if (lang->lint_options.typescript && !allow_typescript) {
return nullptr;
}
if (uri_looks_like_typescript_definition(uri)) {
Path_Classification classified_uri = classify_uri(uri);
if (classified_uri.typescript_definition) {
return &ts_definition;
}
return lang;
Expand Down

0 comments on commit ad6ee67

Please sign in to comment.