Skip to content

Неиспользуемая локальная переменная #920

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 10 commits into from
Closed

Неиспользуемая локальная переменная #920

wants to merge 10 commits into from

Conversation

qtLex
Copy link
Contributor

@qtLex qtLex commented Jan 29, 2020 •
edited
Loading

Описание

Диагностика на неиспользуемые локальные переменные

Переделал диагностику на использование символов.
Расширил список переменных метода создаваемыми через присвоение.
Добавил коллекцию использований в переменную

Связанные задачи

Closes: #15

Чеклист

Общие

  • Ветка PR обновлена из develop
  • Отладочные, закомментированные и прочие, не имеющие смысла участки кода удалены
  • Изменения покрыты тестами
  • Обязательные действия перед коммитом выполнены (запускал команду gradlew precommit)

Для диагностик

  • Описание диагностики заполнено для обоих языков (перевод на английский не обязателен)

Дополнительно

@theshadowco
Copy link
Member

theshadowco commented Jan 31, 2020 •
edited
Loading

Как вариант на подумать: анализировать поблочно

  1. анализировать блок кода, выбирая из него переменные
  2. анализировать использование переменных в этом блоке
  3. анализировать использование переменных блока выше уровнем
  4. выбирать дочерние блоки, повторяя п 1-3

@asosnoviy asosnoviy mentioned this pull request Feb 6, 2020
Open
24 tasks
@qtLex qtLex force-pushed the feature/unused_local_variable branch from ad59978 to e13bec8 Compare April 9, 2020 17:53
@qtLex qtLex changed the title WIP: Неиспользуемая локальная переменная Неиспользуемая локальная переменная Apr 9, 2020
@qtLex
Copy link
Contributor Author

qtLex commented Apr 9, 2020

Снял WIP.
Думаю для первой итерации неплохо получилось, если с символами не намудрил.

@qtLex qtLex force-pushed the feature/unused_local_variable branch from 4d6722e to 4e64b79 Compare April 9, 2020 19:36
artbear
artbear suggested changes Apr 10, 2020
View reviewed changes
src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/VariableSymbol.java
boolean export;
Optional<VariableDescription> description;

ArrayList<Usage> usages = new ArrayList<>();
Copy link
Contributor

Choose a reason for hiding this comment

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

предлагаю List вместо ArrayList

Copy link
Member

Choose a reason for hiding this comment

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

Обязательно использовать List )

src/test/resources/diagnostics/UnusedLocalVariableDiagnostic.bsl
ЛокальнаяСИспользованием = 40;
ТолькоСПрисвоениемЗначения = ВыполнитьДействие(ЛокальнаяСИспользованием);
ВПроцедуреИспользуемая = Проверка();
ВПроцедуреНеИспользуемая = Проверка();
Copy link
Contributor

Choose a reason for hiding this comment

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

а как в коде проверяется. что эта переменная не является переменной формы, реквизитом объекта и т.п. ?
ведь если этих проверок нет, будет ОГРОМНОЕ число ложных срабатываний

Copy link
Member

Choose a reason for hiding this comment

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

вот так https://github.com/1c-syntax/bsl-language-server/pull/920/files#diff-5f2fbbb469ab9b1093959f742dadc232R41

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Модули в которых доступны различного рода реквизиты не анализируются. Как появиться возможность считать из метаданных контекст добавлю фильр

@artbear artbear mentioned this pull request Apr 18, 2020
Open
24 tasks
@nixel2007
Copy link
Member

ждет реализации контекста.

@1c-syntax 1c-syntax deleted a comment from artbear Apr 24, 2020
@1c-syntax 1c-syntax deleted a comment from artbear Apr 28, 2020
@qtLex qtLex force-pushed the feature/unused_local_variable branch from 4e64b79 to 5aea25b Compare May 25, 2020 20:10
@artbear
Copy link
Contributor

artbear commented Feb 24, 2021

@nixel2007 @otymko @qtLex предлагаю не ждать реализации контекстов, прошло уже 10 месяцев и движения пока нет.

а правило хорошее, даже в рамках текущих ограничений.

предлагаю доделать и принимать.

@artbear
Copy link
Contributor

artbear commented Feb 24, 2021

Правило позволяет ловить довольно серьезные ошибки с копипастом или опечатками,
а также полезно при рефакторинге.

@nixel2007
Copy link
Member

@qtLex попробуешь переписать диагностику поверх ReferenceIndex? Стучи в личку, пообщаемся на этот счет, расскажу свои идеи.

@qtLex
Copy link
Contributor Author

qtLex commented Mar 3, 2021

@qtLex попробуешь переписать диагностику поверх ReferenceIndex? Стучи в личку, пообщаемся на этот счет, расскажу свои идеи.

@nixel2007 Ага, попробую. Только сначала гляну на ReferenceIndex, как-то пропустил его.

@nixel2007
Copy link
Member

@qtLex ветка feature/callee-db.
Там много всего, но тебя интересуют в первую очередь сам ReferenceIndex и ReferenceIndexFiller

@nixel2007
Copy link
Member

ReferenceIndex в девелопе

@qtLex
Copy link
Contributor Author

qtLex commented Apr 3, 2021

Переехало в #1611

@qtLex qtLex closed this Apr 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nixel2007 nixel2007 nixel2007 left review comments

@otymko otymko otymko left review comments

+1 more reviewer

@artbear artbear artbear requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Неиспользуемая локальная переменная

5 participants

@qtLex @theshadowco @nixel2007 @artbear @otymko